Skip to content

feat(Algebra/Group/Commutator): tag addCommutatorElement as a scoped instance#38301

Open
SnirBroshi wants to merge 2 commits intoleanprover-community:masterfrom
SnirBroshi:patch-4
Open

feat(Algebra/Group/Commutator): tag addCommutatorElement as a scoped instance#38301
SnirBroshi wants to merge 2 commits intoleanprover-community:masterfrom
SnirBroshi:patch-4

Conversation

@SnirBroshi
Copy link
Copy Markdown
Collaborator

@SnirBroshi SnirBroshi commented Apr 20, 2026

#34784 additivized commutators and made the multiplicative commutator instance available under open scoped commutatorElement.
This makes the additive instance available under open scoped addCommutatorElement.


Should we instead make it scoped under the addCommutatorElement namespace?
The existing usages of additive commutators come from decls marked with open scoped commutatorElement in @[to_additive], and to_additive seemingly doesn't mind that the additivized version is not an instance. So I think they don't have to live in the same namespace.

Open in Gitpod

@SnirBroshi SnirBroshi requested a review from tb65536 April 20, 2026 15:27
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2026

PR summary cc26c21489

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

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).

@github-actions github-actions Bot added the t-algebra Algebra (groups, rings, fields, etc) label Apr 20, 2026
@tb65536
Copy link
Copy Markdown
Contributor

tb65536 commented Apr 20, 2026

I think it should probably be a different namespace, because otherwise open scoped commutatorElement would lead to the same notation being used for two different definitions.

@tb65536 tb65536 added the awaiting-author A reviewer has asked the author a question or requested changes. label Apr 20, 2026
@SnirBroshi SnirBroshi removed the awaiting-author A reviewer has asked the author a question or requested changes. label Apr 20, 2026
@SnirBroshi SnirBroshi requested review from tb65536 and removed request for tb65536 April 20, 2026 17:27
@SnirBroshi
Copy link
Copy Markdown
Collaborator Author

Done, although I think it's very rare to have both Group and AddGroup at the same time (Ring gives GroupWithZero and not Group).

@tb65536
Copy link
Copy Markdown
Contributor

tb65536 commented Apr 20, 2026

Yeah, I know, it just seemed better hygienically to avoid activating them at the same time.

@tb65536
Copy link
Copy Markdown
Contributor

tb65536 commented Apr 20, 2026

Thanks!

maintainer merge

@github-actions
Copy link
Copy Markdown

🚀 Pull request has been placed on the maintainer queue by tb65536.

@mathlib-triage mathlib-triage Bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants