Skip to content

refactor(Algebra): replace RingEquivClass.toRingEquiv by structure-specific coercions#21031

Open
YaelDillies wants to merge 1 commit intomasterfrom
no_generic_hom_coe
Open

refactor(Algebra): replace RingEquivClass.toRingEquiv by structure-specific coercions#21031
YaelDillies wants to merge 1 commit intomasterfrom
no_generic_hom_coe

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor

@YaelDillies YaelDillies commented Jan 24, 2025

Remove the coercion from E to RingEquiv assuming RingEquivClass E and given by RingEquivClass.toRingEquiv. For each concrete type, reimplement this coercion through the relevant structure projection.


Open in Gitpod

@YaelDillies YaelDillies added the WIP Work in progress label Jan 24, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 24, 2025

PR summary b43655dfe2

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ coe_extendScalarsOfSurjective
+ instance : CoeOut (A ≃⋆+* B) (A ≃+* B) where coe := toRingEquiv
+ instance : CoeOut (A₁ ≃ₐ[R] A₂) (A₁ ≃+* A₂) where coe := AlgEquiv.toRingEquiv
+ instance : CoeOut (α ≃+*o β) (α ≃+* β) where coe := toRingEquiv
+ instance : CoeTC (RingInvo R) (R ≃+* Rᵐᵒᵖ) where coe := toRingEquiv
- hasCoeToRingEquiv
- instance [Mul α] [Add α] [Mul β] [Add β] [EquivLike F α β] [RingEquivClass F α β] :

You can run this locally as follows
## summary with just the declaration names:
./scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh contains some details about this script.


No changes to technical debt.

You can run this locally as

./scripts/reporting/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 24, 2025
@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 25, 2025
Copy link
Copy Markdown
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I agree that the generic hom coercions didn't turn out to be particularly useful, so I would be in favour of the PR.

Could you also check that the documentation on how to set up FunLikes is appropriately updated?

@YaelDillies
Copy link
Copy Markdown
Contributor Author

I am not touching this PR for a while by lack of time. Feel free to take over if you feel like it!

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Feb 11, 2025
@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Feb 12, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 11, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 12, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 11, 2025
@leanprover-community-bot-assistant
Copy link
Copy Markdown
Collaborator

This pull request has conflicts, please merge master and resolve them.

@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 11, 2025
@urkud
Copy link
Copy Markdown
Member

urkud commented Apr 11, 2025

Please add some explanations to the commit message.

YaelDillies added a commit that referenced this pull request May 11, 2025
It's a big issue that this coercion even exists, but it is difficult to remove at the moment, cf #21031.
mathlib-bors bot pushed a commit that referenced this pull request May 11, 2025
It's a big issue that this coercion even exists, but it is difficult to remove at the moment, cf #21031.

From Toric
tannerduve pushed a commit that referenced this pull request May 13, 2025
It's a big issue that this coercion even exists, but it is difficult to remove at the moment, cf #21031.

From Toric
bwehlin pushed a commit to bwehlin/mathlib4 that referenced this pull request May 31, 2025
…rover-community#24779)

It's a big issue that this coercion even exists, but it is difficult to remove at the moment, cf leanprover-community#21031.

From Toric
@mathlib4-merge-conflict-bot
Copy link
Copy Markdown
Collaborator

This pull request has conflicts, please merge master and resolve them.

@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 11, 2025
@Vierkantor Vierkantor self-assigned this Oct 23, 2025
@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 19, 2025
@mathlib4-merge-conflict-bot
Copy link
Copy Markdown
Collaborator

This pull request has conflicts, please merge master and resolve them.

@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 19, 2025
@Vierkantor
Copy link
Copy Markdown
Contributor

What is the status of this PR? Because this is work that we want to get into Mathlib at some point, but I've been holding off on reviewing because of the empty PR description and WIP tag.

@YaelDillies
Copy link
Copy Markdown
Contributor Author

The status is that it needs a lot more work before it's reviewable. I'm expecting the final diff to be around +-3k lines.

@j-loreaux suggested preliminary steps to this PR but I am not sure that one can actually take those steps before the whole PR.

Concretely, he suggests to first remove uses of hom classes in definitions, and then remove the generic coercions. The issue I see is that this will change the syntactic form of a lot of lemmas, and simp won't yet have been upgraded to handle that. My suggestion instead is to do the refactor one type of morphisms at a time, starting from the leaves of the morphism hierarchy and working towards the roots.

@mathlib4-merge-conflict-bot mathlib4-merge-conflict-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 1, 2026
@mathlib4-merge-conflict-bot
Copy link
Copy Markdown
Collaborator

This pull request has conflicts, please merge master and resolve them.

@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 1, 2026
@mathlib-merge-conflicts mathlib-merge-conflicts bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Feb 4, 2026
@mathlib-merge-conflicts
Copy link
Copy Markdown

This pull request has conflicts, please merge master and resolve them.

@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Feb 4, 2026
@YaelDillies YaelDillies changed the title chore: get rid of generic hom coercions refactor(Algebra): replace RingEquivClass.toRingEquiv by structure-specific coercions Apr 8, 2026
@YaelDillies YaelDillies added t-algebra Algebra (groups, rings, fields, etc) and removed WIP Work in progress labels Apr 8, 2026
@YaelDillies YaelDillies changed the title refactor(Algebra): replace RingEquivClass.toRingEquiv by structure-specific coercions refactor(Algebra): replace RingEquivClass.toRingEquiv by structure-specific coercions Apr 9, 2026
@mathlib-merge-conflicts mathlib-merge-conflicts bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 11, 2026
@mathlib-merge-conflicts
Copy link
Copy Markdown

This pull request has conflicts, please merge master and resolve them.

…specific coercions

Remove the coercion from `E` to `RingEquiv` assuming `RingEquivClass E` and given by `RingEquivClass.toRingEquiv`.
For each concrete type, reimplement this coercion through the relevant structure projection.
@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants