Skip to content

Standardise implicit arguments of cancellation properties. #1436

Open
@FR-vdash-bot

Description

@FR-vdash-bot

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.

Activity

added this to the v2.0 milestone on Feb 24, 2021
MatthewDaggitt

MatthewDaggitt commented on Feb 24, 2021

@MatthewDaggitt
Contributor

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.

FR-vdash-bot

FR-vdash-bot commented on Feb 24, 2021

@FR-vdash-bot
ContributorAuthor

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
MatthewDaggitt

MatthewDaggitt commented on Sep 12, 2022

@MatthewDaggitt
Contributor

Okay so

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

MatthewDaggitt commented on Sep 12, 2022

@MatthewDaggitt
Contributor

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.

JacquesCarette

JacquesCarette commented on Sep 16, 2022

@JacquesCarette
Contributor

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.

jamesmckinna

jamesmckinna commented on Sep 25, 2023

@jamesmckinna
Contributor

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.

27 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Participants

      @JacquesCarette@MatthewDaggitt@FR-vdash-bot@jamesmckinna

      Issue actions

        Standardise implicit arguments of cancellation properties. · Issue #1436 · agda/agda-stdlib