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

[Merged by Bors] - fix(NumberTheory/LegendreSymbol/AddCharacter): Disentangle coercion mess for AddChar #8803

Closed
wants to merge 4 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Dec 3, 2023

There are two things going on here:

  • AddChar G R has two syntactically different coercions to function G → R:
    • FunLike.coe via AddChar.monoidHomClass
    • AddChar.toFun via AddChar.hasCoeToFun
  • AddChar.hasCoeToFun and AddChar.monoidHomClass together create a diamond for the typeclass problem FunLike (AddChar G R) _ _

This PR fixes both problems by

  • removing the HasCoeToFun instance and the corresponding AddChar.toFun
  • changing MonoidHomClass (AddChar G R) (Multiplicative G) R to FunLike (AddChar G R) G fun _ ↦ R (isn't the whole point of AddChar to go from an additive group to a multiplicative one anyway?)

This PR isn't meant to be perfect. It is a stopgag to an issue that has completely startled progress on LeanAPAP. Once it is fixed, a much more thorough refactor will follow.

This breaks a rewrite downstream that now unifies to some defeq but not syntactically equal type.
This is more an indication of fragility in the original proof.


Open in Gitpod

There are two things going on here:
* `AddChar G R` has two syntactically different coercions to function `G → R`:
  * `FunLike.coe` via `AddChar.monoidHomClass`
  * `AddChar.toFun` via `AddChar.hasCoeToFun`
* `AddChar.hasCoeToFun` and `AddChar.monoidHomClass` together create a diamond for the typeclass problem `FunLike (AddChar G R) _ _`

This PR fixes both problems by
* removing the `HasCoeToFun` instance and the corresponding `AddChar.toFun`
* changing `MonoidHomClass (AddChar G R) (Multiplicative G) R` to `FunLike (AddChar G R) G fun _ ↦ R` (isn't the whole point of `AddChar` to go from an additive group to a multiplicative one anyway?)

This PR isn't meant to be perfect. It is a stopgag to an issue that has completely startled progress on LeanAPAP. Once it is fixed, a much more thorough refactor will follow.
@YaelDillies YaelDillies added the awaiting-review The author would like community review of the PR label Dec 3, 2023
@eric-wieser eric-wieser changed the title fix: Disentangle coercion mess in AddChar fix(NumberTheory/LegendreSymbol/AddCharacter): Disentangle coercion mess for AddChar Dec 3, 2023
YaelDillies and others added 2 commits December 3, 2023 23:13
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@eric-wieser
Copy link
Member

bors d+

Just a comment request for the failing rw fix.

@mathlib-bors
Copy link

mathlib-bors bot commented Dec 3, 2023

✌️ YaelDillies can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added delegated and removed awaiting-review The author would like community review of the PR labels Dec 3, 2023
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@YaelDillies
Copy link
Collaborator Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Dec 4, 2023
…ess for `AddChar` (#8803)

There are two things going on here:
* `AddChar G R` has two syntactically different coercions to function `G → R`:
  * `FunLike.coe` via `AddChar.monoidHomClass`
  * `AddChar.toFun` via `AddChar.hasCoeToFun`
* `AddChar.hasCoeToFun` and `AddChar.monoidHomClass` together create a diamond for the typeclass problem `FunLike (AddChar G R) _ _`

This PR fixes both problems by
* removing the `HasCoeToFun` instance and the corresponding `AddChar.toFun`
* changing `MonoidHomClass (AddChar G R) (Multiplicative G) R` to `FunLike (AddChar G R) G fun _ ↦ R` (isn't the whole point of `AddChar` to go from an additive group to a multiplicative one anyway?)

This PR isn't meant to be perfect. It is a stopgag to an issue that has completely startled progress on LeanAPAP. Once it is fixed, a much more thorough refactor will follow.

This breaks a rewrite downstream that now unifies to some defeq but not syntactically equal type.
This is more an indication of fragility in the original proof.
@mathlib-bors
Copy link

mathlib-bors bot commented Dec 4, 2023

Build failed (retrying...):

mathlib-bors bot pushed a commit that referenced this pull request Dec 4, 2023
…ess for `AddChar` (#8803)

There are two things going on here:
* `AddChar G R` has two syntactically different coercions to function `G → R`:
  * `FunLike.coe` via `AddChar.monoidHomClass`
  * `AddChar.toFun` via `AddChar.hasCoeToFun`
* `AddChar.hasCoeToFun` and `AddChar.monoidHomClass` together create a diamond for the typeclass problem `FunLike (AddChar G R) _ _`

This PR fixes both problems by
* removing the `HasCoeToFun` instance and the corresponding `AddChar.toFun`
* changing `MonoidHomClass (AddChar G R) (Multiplicative G) R` to `FunLike (AddChar G R) G fun _ ↦ R` (isn't the whole point of `AddChar` to go from an additive group to a multiplicative one anyway?)

This PR isn't meant to be perfect. It is a stopgag to an issue that has completely startled progress on LeanAPAP. Once it is fixed, a much more thorough refactor will follow.

This breaks a rewrite downstream that now unifies to some defeq but not syntactically equal type.
This is more an indication of fragility in the original proof.
@mathlib-bors
Copy link

mathlib-bors bot commented Dec 4, 2023

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(NumberTheory/LegendreSymbol/AddCharacter): Disentangle coercion mess for AddChar [Merged by Bors] - fix(NumberTheory/LegendreSymbol/AddCharacter): Disentangle coercion mess for AddChar Dec 4, 2023
@mathlib-bors mathlib-bors bot closed this Dec 4, 2023
@mathlib-bors mathlib-bors bot deleted the add_char_coe branch December 4, 2023 13:54
awueth pushed a commit that referenced this pull request Dec 19, 2023
…ess for `AddChar` (#8803)

There are two things going on here:
* `AddChar G R` has two syntactically different coercions to function `G → R`:
  * `FunLike.coe` via `AddChar.monoidHomClass`
  * `AddChar.toFun` via `AddChar.hasCoeToFun`
* `AddChar.hasCoeToFun` and `AddChar.monoidHomClass` together create a diamond for the typeclass problem `FunLike (AddChar G R) _ _`

This PR fixes both problems by
* removing the `HasCoeToFun` instance and the corresponding `AddChar.toFun`
* changing `MonoidHomClass (AddChar G R) (Multiplicative G) R` to `FunLike (AddChar G R) G fun _ ↦ R` (isn't the whole point of `AddChar` to go from an additive group to a multiplicative one anyway?)

This PR isn't meant to be perfect. It is a stopgag to an issue that has completely startled progress on LeanAPAP. Once it is fixed, a much more thorough refactor will follow.

This breaks a rewrite downstream that now unifies to some defeq but not syntactically equal type.
This is more an indication of fragility in the original proof.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants