Skip to content

refactor(GroupTheory/Submonoid/Operations): Use SubmonoidClass and delete redundant instances - #11304

Closed
tb65536 wants to merge 14 commits into
masterfrom
tb_monoid_action
Closed

refactor(GroupTheory/Submonoid/Operations): Use SubmonoidClass and delete redundant instances#11304
tb65536 wants to merge 14 commits into
masterfrom
tb_monoid_action

Conversation

@tb65536

@tb65536 tb65536 commented Mar 11, 2024

Copy link
Copy Markdown
Contributor

This PR refactors some SMul instances in GroupTheory/Submonoid/Operations.lean to use SubmonoidClass. This makes many other similar instances redundant.


Open in Gitpod

@tb65536 tb65536 added awaiting-review awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. t-algebra Algebra (groups, rings, fields, etc) labels Mar 11, 2024
@github-actions github-actions Bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Mar 12, 2024
@jcommelin

Copy link
Copy Markdown
Member

This seems like a good move. Let's also bench it.

!bench

@jcommelin

Copy link
Copy Markdown
Member

!bench

@leanprover-bot

Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit b3d2126.
There were significant changes against commit 68f3af6:

  Benchmark                     Metric         Change
  ===================================================
- build                         wall-clock      22.2%
- ~Mathlib.Algebra.Module.PID   instructions     5.7%

@eric-wieser

Copy link
Copy Markdown
Member

Maybe not so good an idea!

@jcommelin

Copy link
Copy Markdown
Member

@tb65536 Given this bench result, what do you think? Do you want to attempt to improve performance, or shall we punt on this?

@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Mar 26, 2024
@ghost ghost 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 28, 2024
@tb65536

tb65536 commented Mar 28, 2024

Copy link
Copy Markdown
Contributor Author

Probably just punt for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-author A reviewer has asked the author a question or requested changes. merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants