Skip to content

[Merged by Bors] - fix(Mathlib/Algebra/Lie/DirectSum): remove unused R argument from lemmas#8388

Closed
eric-wieser wants to merge 5 commits intomasterfrom
eric-wieser/lie-direct_sum
Closed

[Merged by Bors] - fix(Mathlib/Algebra/Lie/DirectSum): remove unused R argument from lemmas#8388
eric-wieser wants to merge 5 commits intomasterfrom
eric-wieser/lie-direct_sum

Conversation

@eric-wieser
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser commented Nov 13, 2023

This made them not actually work as a simp lemma.

Also extracts a common result that can be used to prove single_add for DFinsupp and Finsupp, and a new Finsupp.single_mul lemma.


Open in Gitpod

…emmas

This made them not actually work as a `simp` lemma
@eric-wieser eric-wieser added awaiting-review awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Nov 13, 2023
@eric-wieser eric-wieser requested a review from ocfnash November 13, 2023 14:59
Copy link
Copy Markdown
Contributor

@ocfnash ocfnash left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. I'm surprised this wasn't caught by a linter (variable used in proof, not used in statement).

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Nov 13, 2023

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

@ghost ghost added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Nov 13, 2023
@eric-wieser eric-wieser added the t-algebra Algebra (groups, rings, fields, etc) label Nov 13, 2023
@eric-wieser
Copy link
Copy Markdown
Member Author

I ended up with a small bit of scope creep when responding to review comments; mind taking another look @ocfnash?

@ocfnash
Copy link
Copy Markdown
Contributor

ocfnash commented Nov 13, 2023

Looks great, thanks!

bors merge

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Nov 13, 2023

👎 Rejected by label

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Nov 13, 2023
@ocfnash ocfnash removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Nov 13, 2023
@ocfnash
Copy link
Copy Markdown
Contributor

ocfnash commented Nov 13, 2023

bors is just being very conservative (it's passed everything except for lean4checker)

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Nov 13, 2023

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

@eric-wieser
Copy link
Copy Markdown
Member Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Nov 13, 2023
…emmas (#8388)

This made them not actually work as a `simp` lemma.

Also extracts a common result that can be used to prove `single_add` for `DFinsupp` and `Finsupp`, and a new `Finsupp.single_mul` lemma.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Nov 13, 2023

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(Mathlib/Algebra/Lie/DirectSum): remove unused R argument from lemmas [Merged by Bors] - fix(Mathlib/Algebra/Lie/DirectSum): remove unused R argument from lemmas Nov 13, 2023
@mathlib-bors mathlib-bors bot closed this Nov 13, 2023
@mathlib-bors mathlib-bors bot deleted the eric-wieser/lie-direct_sum branch November 13, 2023 18:56
@alreadydone
Copy link
Copy Markdown
Contributor

Thanks for doing this. I'm surprised this wasn't caught by a linter (variable used in proof, not used in statement).

What linter could possibly catch this? Certainly statements could require additional structure to hold, for example m + m = 0 for all m in an AddCommGroup M if it's equipped with a Module (ZMod 2) M structure. For the same reason, your lemmas may well require a R-Lie module structure on each L i!

@eric-wieser
Copy link
Copy Markdown
Member Author

The simp linter should have caught that the simp lemma can't actually be proved with simp (and after #7905 it does)

@ocfnash
Copy link
Copy Markdown
Contributor

ocfnash commented Nov 14, 2023

You are both absolutely right, thanks @eric-wieser and @alreadydone !

alexkeizer pushed a commit that referenced this pull request Nov 17, 2023
…emmas (#8388)

This made them not actually work as a `simp` lemma.

Also extracts a common result that can be used to prove `single_add` for `DFinsupp` and `Finsupp`, and a new `Finsupp.single_mul` lemma.
alexkeizer pushed a commit that referenced this pull request Nov 21, 2023
…emmas (#8388)

This made them not actually work as a `simp` lemma.

Also extracts a common result that can be used to prove `single_add` for `DFinsupp` and `Finsupp`, and a new `Finsupp.single_mul` lemma.
grunweg pushed a commit that referenced this pull request Dec 15, 2023
…emmas (#8388)

This made them not actually work as a `simp` lemma.

Also extracts a common result that can be used to prove `single_add` for `DFinsupp` and `Finsupp`, and a new `Finsupp.single_mul` lemma.
mathlib-bors bot pushed a commit that referenced this pull request Jan 10, 2025
…20592)

Uses the [`lie_of_same`](https://github.com/leanprover-community/mathlib4/blob/d4891b46ecc82d3f6050dd51c2699fee91a2347b/Mathlib/Algebra/Lie/DirectSum.lean#L118) lemma added in #8388 to shorten a 13-line proof down to a single line, removing two `erw`s and one porting note.

According to `set_option trace.profiler true`, the new `lieAlgbraOf` declaration takes 135 milliseconds to elaborate, compared to 298 milliseconds for the declaration it replaces.

Found via [`tryAtEachStep`](https://github.com/dwrensha/tryAtEachStep).
grunweg pushed a commit that referenced this pull request Jan 11, 2025
…20592)

Uses the [`lie_of_same`](https://github.com/leanprover-community/mathlib4/blob/d4891b46ecc82d3f6050dd51c2699fee91a2347b/Mathlib/Algebra/Lie/DirectSum.lean#L118) lemma added in #8388 to shorten a 13-line proof down to a single line, removing two `erw`s and one porting note.

According to `set_option trace.profiler true`, the new `lieAlgbraOf` declaration takes 135 milliseconds to elaborate, compared to 298 milliseconds for the declaration it replaces.

Found via [`tryAtEachStep`](https://github.com/dwrensha/tryAtEachStep).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants