Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the makeX functions but keep the makeIsX #598

Open
mortberg opened this issue Aug 23, 2021 · 4 comments
Open

Remove the makeX functions but keep the makeIsX #598

mortberg opened this issue Aug 23, 2021 · 4 comments

Comments

@mortberg
Copy link
Collaborator

We could remove for example makeGroup as groups should be constructed using copatterns anyway. It might still be nice to keep the makeIsGroup and it should hopefully not be a problem as the copatterns in the group def will block the laws from getting unfolded unless necessary

@barrettj12
Copy link
Contributor

I disagree, since I think makeX gives much more readable definitions. E.g. looking at the current definition of the group ℤ in Cubical.Algebra.Group.Instances.Int:

: Group₀
fst ℤ = ℤType
1g (snd ℤ) = 0
_·_ (snd ℤ) = _+ℤ_
inv (snd ℤ) = _-ℤ_ 0
isGroup (snd ℤ) = isGroupℤ
  where
  abstract
    isGroupℤ : IsGroup (pos 0) _+ℤ_ (_-ℤ_ (pos 0))
    isGroupℤ = makeIsGroup isSetℤ +Assoc (λ _  refl) (+Comm 0)
                           (λ x  +Comm x (pos 0 -ℤ x) ∙ minusPlus x 0)
                           (λ x  minusPlus x 0)

vs using makeGroup:

= makeGroup
  0
  _+ℤ_
  (_-ℤ_ 0)
  isSetℤ
  +Assoc
  (λ _  refl)
  (+Comm 0)
  (λ x  +Comm x (pos 0 -ℤ x) ∙ minusPlus x 0)
  (λ x  minusPlus x 0)

You could even annotate it:

= makeGroup
  0          -- identity
  _+ℤ_       -- group operation
  (_-ℤ_ 0)   -- inverses
  ...

@barrettj12
Copy link
Contributor

On the other hand, I just tried doing this with the GroupStr constructor groupstr:

: Group₀
ℤ = ℤType , groupstr
      0
      _+ℤ_
      (_-ℤ_ 0)
      (makeIsGroup 
        isSetℤ
        +Assoc
        (λ _  refl)
        (+Comm 0)
        (λ x  +Comm x (pos 0 -ℤ x) ∙ minusPlus x 0)
        (λ x  minusPlus x 0)
      )

which looks very similar. Probably not all of these are necessary to keep.

I think some of the makeX functions (e.g. makeGroup-left, makeGroup-right and makeAbGroup) are worth keeping because they allow you to provide only a minimal amount of structure when the rest can be inferred.

@mortberg
Copy link
Collaborator Author

mortberg commented Nov 21, 2021

It's mainly a matter of efficiency. Copatterns have the benefit that Agda won't unfold the definitions unless one applies a projection. This seems completely essential to ensure reasonable typechecking times for complex constructions (e.g. Z cohomology groups). By now I also find the copattern definitions more readable as well. If you have

ℤ : Group₀
fst ℤ = ℤType
1g (snd ℤ) = 0
_·_ (snd ℤ) = _+ℤ_
inv (snd ℤ) = _-ℤ_ 0
...

you don't need any comments about what is the 1, operation and inverse are

So I think all algebra structure operations should be given by copatterns, but the proofs that these operations form an instance of the structure can be given using the makeX (and then also maybe made opaque using abstract)

@barrettj12
Copy link
Contributor

If X / isX is a record type, then can't we just use a constructor instead of makeX / makeIsX?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants