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

chore (Algebra.Basic): make RingHom.id and related declarations reducible #9025

Closed
wants to merge 11 commits into from

Conversation

mattrobball
Copy link
Collaborator

@mattrobball mattrobball commented Dec 13, 2023

These instances often appear as parameters to typeclass instances that need to unify reducibly.


Open in Gitpod

@mattrobball
Copy link
Collaborator Author

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit ca5cf29.
There were significant changes against commit d85330e:

  Benchmark                                                  Metric          Change
  =================================================================================
- build                                                      instructions      5.7%
- build                                                      simp             29.6%
- build                                                      wall-clock        8.2%
- ~Mathlib.Algebra.Lie.BaseChange                            instructions   2789.9%
- ~Mathlib.Algebra.Lie.Killing                               instructions    211.7%
- ~Mathlib.AlgebraicGeometry.ProjectiveSpectrum.Scheme       instructions     52.3%
- ~Mathlib.Analysis.Calculus.LagrangeMultipliers             instructions    383.8%
- ~Mathlib.Analysis.InnerProductSpace.Adjoint                instructions      3.9%
- ~Mathlib.Analysis.NormedSpace.Extend                       instructions    154.7%
- ~Mathlib.Analysis.NormedSpace.Multilinear.Basic            instructions      3.6%
- ~Mathlib.Analysis.SpecialFunctions.PolarCoord              instructions    467.2%
- ~Mathlib.Analysis.SpecialFunctions.Pow.Deriv               instructions    369.1%
- ~Mathlib.Data.Polynomial.Derivation                        instructions    131.0%
- ~Mathlib.Geometry.Manifold.Instances.Sphere                instructions      5.0%
- ~Mathlib.LinearAlgebra.AffineSpace.AffineMap               instructions     29.2%
- ~Mathlib.LinearAlgebra.Basis                               instructions    594.4%
- ~Mathlib.LinearAlgebra.BilinearForm.Hom                    instructions     52.9%
- ~Mathlib.LinearAlgebra.Matrix.BilinearForm                 instructions    304.7%
- ~Mathlib.LinearAlgebra.Matrix.SesquilinearForm             instructions    217.5%
- ~Mathlib.LinearAlgebra.Prod                                instructions    159.5%
- ~Mathlib.RepresentationTheory.Character                    instructions    803.6%
- ~Mathlib.RepresentationTheory.GroupCohomology.Basic        instructions     53.3%
- ~Mathlib.RepresentationTheory.GroupCohomology.Resolution   instructions      2.9%
- ~Mathlib.RingTheory.PowerSeries.Basic                      instructions     28.8%

Comment on lines 434 to 435
instance id : Algebra R R :=
@[reducible] instance id : Algebra R R :=
Copy link
Member

Choose a reason for hiding this comment

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

I thought the whole point in instance is that it's reducible enough for anything typeclass related?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Apparently not

Copy link
Member

Choose a reason for hiding this comment

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

Do you have a mwe to demo that?

(I agree with the principle behind the rest of this PR to change the non-instances to be reducible)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah processing what you are saying now. You are correct, that is silly.

@mattrobball mattrobball changed the title chore (Algebra.Basic): make Algebra.id, RingHom.id, MonoidHom.id reducible chore (Algebra.Basic): make RingHom.id and related declarations reducible Dec 14, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-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 Dec 23, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-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 12, 2024
@mattrobball mattrobball deleted the mrb/reducible_algebra_id branch January 24, 2024 03:22
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

Successfully merging this pull request may close these issues.

4 participants