agda-stdlib icon indicating copy to clipboard operation
agda-stdlib copied to clipboard

Standardise implicit arguments of cancellation properties.

Open astrainfinita opened this issue 4 years ago • 20 comments

In Algebra.Definitions

LeftCancellative _•_ = ∀ x {y z} → (x • y) ≈ (x • z) → y ≈ z
RightCancellative _•_ = ∀ {x} y z → (y • x) ≈ (z • x) → y ≈ z
AlmostLeftCancellative e _•_ = ∀ {x} y z → ¬ x ≈ e → (x • y) ≈ (x • z) → y ≈ z
AlmostRightCancellative e _•_ = ∀ {x} y z → ¬ x ≈ e → (y • x) ≈ (z • x) → y ≈ z

In Data.Rational.Unnormalised.Properties

+-cancelˡ : ∀ {r p q} → r + p ≃ r + q → p ≃ q
+-cancelʳ : ∀ {r p q} → p + r ≃ q + r → p ≃ q

Perhaps we should make them uniform.

astrainfinita avatar Feb 23 '21 19:02 astrainfinita

Hey, thanks for the spot! Yup, so for starters the rational proofs should be using the LeftCancellative and RightCancellative types.

As for the AlmostX and X definitions, the reason for the differences in the implicits is Agda's ability to infer them. LeftCancellative and RightCancellative were introduced early on in the library's life and the implicit arguments are slightly broken, see the code below. To fix this I think none of the arguments to those definitions should be implicit. The Almost versions were introduced very recently and I think have the right implicits (x is inferrable from the equality).

open import Data.Rational.Base
open import Algebra.Definitions
open import Relation.Binary.PropositionalEquality.Core

postulate
  rc : RightCancellative _≡_ _+_
  arc : AlmostRightCancellative _≡_ 0ℚ _+_
  x y z : ℚ
  x≢0 : x ≢ 0ℚ
  eq : y + x ≡ z + x

-- Inference error
test : y ≡ z
test = rc y z eq

-- No inference error
test2 : y ≡ z
test2 = arc y z x≢0 eq

Therefore I think the correct definitions are:

LeftCancellative _•_ = ∀ x y z → (x • y) ≈ (x • z) → y ≈ z
RightCancellative _•_ = ∀ x y z → (y • x) ≈ (z • x) → y ≈ z
AlmostLeftCancellative e _•_ = ∀ {x} y z → ¬ x ≈ e → (x • y) ≈ (x • z) → y ≈ z
AlmostRightCancellative e _•_ = ∀ {x} y z → ¬ x ≈ e → (y • x) ≈ (z • x) → y ≈ z

but we will need to wait until v2.0 to make such a breaking change.

MatthewDaggitt avatar Feb 24 '21 01:02 MatthewDaggitt

I have just noticed something similar.

In Data.Nat.Properties

≤-stepsˡ : ∀ {m n} o → m ≤ n → m ≤ o + n
≤-stepsʳ : ∀ {m n} o → m ≤ n → m ≤ n + o
m≤m+n : ∀ m n → m ≤ m + n
m≤n+m : ∀ m n → m ≤ n + m

In Data.Integer.Properties

≤-steps : ∀ {m n} p → m ≤ n → m ≤ + p + n
m≤m+n : ∀ {m} n → m ≤ m + + n
n≤m+n : ∀ m {n} → n ≤ + m + n

In Data.Rational.Unnormalised.Properties

≤-steps : ∀ {p q r} → NonNegative r → p ≤ q → p ≤ r + q
p≤p+q : ∀ {p q} → NonNegative q → p ≤ p + q
p≤q+p : ∀ {p} → NonNegative p → ∀ {q} → q ≤ p + q

astrainfinita avatar Feb 24 '21 06:02 astrainfinita

Okay so

  • [x] we should be explicit everywhere in Algebra.Definitions
  • [ ] we should change Data.Rational.Properties (and elsewhere) to be use those definitions

MatthewDaggitt avatar Sep 12 '22 14:09 MatthewDaggitt

We should check that the subsequent inconsistencies in Data.Nat/Integer/Rational.Properties been resolved by the NonZero rewrite everywhere, as their types have changed.

MatthewDaggitt avatar Sep 12 '22 14:09 MatthewDaggitt

So there's a problem with using the definitions in Data.Rational.Properties: the notion of NonZero that it uses is different than the notion that AlmostLeftCancellative uses. So I'm not sure we can make that change. To make things consistent, what we'd need to do is to parametrize AlmostLeftCancellative by a "non zero" predicate.

Also, by inconsistencies, do you mean like the ones pointed out above regarding implicit/explicit? For example, in ≤-stepsˡ it does make sense to make them implicit, as they'll always be inferable. So I'm not quite sure what to do for part 2 of this.

JacquesCarette avatar Sep 16 '22 12:09 JacquesCarette

Is there a common resolution available of this issue with (the implied need for non-zeroness distinct from NonZero brought up by) #1562? And #1175?

Eg. see developing branch towards a possible PR...

... where I will shortly push some breaking (and speculative) changes.

jamesmckinna avatar Sep 25 '23 07:09 jamesmckinna

While I'm assigned this (and I don't mind), right now, I'm not quite sure what ought to be done. It feels like enough things have "moved underneath" that I'm not sure what is feasible / outstanding. Especially given some of @jamesmckinna 's work in #1562 , #1579 and #2117 .

JacquesCarette avatar Apr 24 '24 01:04 JacquesCarette

Fair enough! Plus I have mentally timed out on a lot of this issue since those PRs, and more importantly, the post-v2.0 cycle of development I'm currently working on...

Ping me on Zulip if you want me to try to page this back in...

jamesmckinna avatar May 07 '24 12:05 jamesmckinna

[Summarising comments I made to @JacquesCarette on Zulip]

  • I think #2117 was a bit too ambitious in trying to reconcile the Monotone and Cancellative families of properties, so that should be abandoned (until at least v3.0 if not beyond...)
  • I agree with the (existing) approach of having the Almost* properties defined in terms of the (negation of the) underlying equality, and hence with any agreed version of what the implicit/explicit quantification should be, as discussed above

But that said, I think that the concrete arithmetic datatypes present a different challenge, namely that the corresponding Almost* properties (probably) also need separate versions where the ¬ x ≈ e precondition is replaced by .{{ NonZero x }}... because

  • the mode-of-use of NonZero for the numeric types is typically as an (irrelevant) instance (eg paradigmatically for this issue the already-defined *-cancelʳ-≡ : ∀ m n o .{{_ : NonZero o}} → m * o ≡ n * o → m ≡ n in Data.Nat.Properties)
  • Agda, AFAIK, doesn't admit a form of property statement which would allow the above, and ∀ m n o → o ≢ 0 → m * o ≡ n * o → m ≡ n to be specific instantiations of some hypothetical common AlmostRightCancellativeP : Pred A _ → Op₂ A → Set _ AlmostRightCancellativeP P _•_ = ∀ x y z → P x → (y • x) ≈ (z • x) → y ≈ z, because of the instance brackets... (the irrelevance annotation can be handled separately via recompute; see below)

So: it seems as though two parallel strands of definitions are required, one with the quantified instance, one with the _ ≉ 0# precondition... which does seems annoying, even if we prove a lemma (in Algebra.Consequences) of the form relating the two definitions when there is an ambient proof that NonZero x → x ≉ 0#... as is the case for all the concrete datatypes (cf. Biased implementations in the Algebra hierarchy)

Worse (as regards duplication) it is going to be typically again the case that the .{{ NonZero ... }} version will be the one that clients will want to use for the concrete types, in order to exploit instance-based reasoning for 'boring' predicates such as NonZero... but from which behaviour abstract algebraic structures won't be able to profit.

The only conceivable (to me, at least ;-)) unification (in terms of a hypothetical AlmostRightCancellativeP as above) might be to take P = NonZero for the concrete datatypes, and then use Relation.Nullary.Decidable.Core.recompute gymnastics to turn such instantiations into the corresponding irrelevant instance-based statement, because all the relevant concrete NonZero predicates are decidable.

But that still doesn't solve the problem of such derived statements nevertheless needing a consistent form... and we are/seem to be back to duplication of the statement (template) again... sigh.

jamesmckinna avatar May 18 '24 12:05 jamesmckinna

One possible alternative statement, but which then still requires some specialisation for the concrete case, would be to rephrase AlmostCancellative in 'positive' constructive style, viz. as ∀ x y z → (x ≈ e) ⊎ (x • y) ≈ (x • z) → y ≈ z (and thereby avoiding @JacquesCarette 's aversion to negated Setoid equality as a code smell)... but that still doesn't seem to avoid the duplication problem...

jamesmckinna avatar May 18 '24 12:05 jamesmckinna

Unless, perhaps, we...

  • either bite the bullet, and only accept AlmostCancellative statements for concrete algebras (seems bad)
  • or, introduce new Algebra.Structures... (apologies for the sketchiness of the sketch)
record NonZero (0# : A) : Set _ where
  field
    NonZero : Pred A _
    nonZero-≢ : NonZero ⇒ (_≉ 0#)
    nonZero-≢⁻¹ : (_≉ 0#) ⇒ NonZero

record DecNonZero (0# : A) : Set _ where
  field
    nonZero : NonZero 0#
    nonZero? : Unary.Decidable NonZero

record AlmostCancellative (_∙_ : Op₂ A) : Set _ where
  field
    cancellative : AlmostCancellative _∙_

module NonZeroCancellative (NZ : NonZero 0#) where

  record AlmostCancellativeNZ (_∙_ : Op₂ A) : Set _ where
    field
      cancellative : AlmostCancellativeNZ _∙_ -- '.{{NonZero x}} version'

    almostCancellative : AlmostCancellative _∙_
    almostCancellative = ...

etc. ? (Modulo better choices of names... apologies for any unintended confusion potentially introduced by poor choice of names above... TL;DR: instead of Consequences, introduced a record which reified possession of a sound-and-complete NonZero implementation, plus a record which witnesses each kind of the AlmostCancellable property, with one having a Biased-style implementation of the other...)

jamesmckinna avatar May 18 '24 17:05 jamesmckinna

Yes, I think at this point I don't see how to avoid duplication... It's all very unsatisfying and I feel that we're missing something. As a hunch, I would say we're running up into the lack of ability to be generic over modalities (e.g. irrelevance)....

MatthewDaggitt avatar Jul 11 '24 02:07 MatthewDaggitt