Skip to content

perf: improve some instances in MvPolynomial#7436

Closed
ChrisHughes24 wants to merge 3 commits intomasterfrom
MvPolyPerfCH
Closed

perf: improve some instances in MvPolynomial#7436
ChrisHughes24 wants to merge 3 commits intomasterfrom
MvPolyPerfCH

Conversation

@ChrisHughes24
Copy link
Copy Markdown
Member

@ChrisHughes24 ChrisHughes24 commented Sep 29, 2023


Open in Gitpod

@ChrisHughes24 ChrisHughes24 added WIP Work in progress awaiting-author A reviewer has asked the author a question or requested changes. labels Sep 29, 2023
@ChrisHughes24
Copy link
Copy Markdown
Member Author

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 10e1279.
There were significant changes against commit 042888f:

  Benchmark                                       Metric         Change
  =====================================================================
+ ~Mathlib.Data.MvPolynomial.Funext               instructions    -6.2%
- ~Mathlib.NumberTheory.LegendreSymbol.GaussSum   instructions     6.4%
+ ~Mathlib.RingTheory.FinitePresentation          instructions   -27.0%
+ ~Mathlib.RingTheory.Jacobson                    instructions    -5.0%
- ~Mathlib.RingTheory.MvPolynomial.Basic          instructions     5.8%
+ ~Mathlib.RingTheory.Polynomial.Quotient         instructions   -13.1%
+ ~Mathlib.RingTheory.WittVector.WittPolynomial   instructions    -7.3%

@ChrisHughes24 ChrisHughes24 added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Sep 30, 2023
@ChrisHughes24 ChrisHughes24 added awaiting-review and removed WIP Work in progress blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Oct 4, 2023
@ghost ghost added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 4, 2023
@ChrisHughes24 ChrisHughes24 removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 4, 2023
@ghost ghost added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 4, 2023
@ChrisHughes24 ChrisHughes24 removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 4, 2023
Copy link
Copy Markdown
Contributor

@mattrobball mattrobball left a comment

Choose a reason for hiding this comment

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

It is tedious but building all the intermediate instances for hierarchy should be best for performance.

AddMonoidAlgebra.commRing
--TODO: add reference to library note in PR #7432
{ AddMonoidAlgebra.commRing with
toSemiring := MvPolynomial.commSemiring.toSemiring }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there no Ring instance? This seems problematic to unify.

@ChrisHughes24 ChrisHughes24 added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Oct 25, 2023
@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 Apr 5, 2024
@YaelDillies YaelDillies deleted the MvPolyPerfCH branch July 31, 2025 11:39
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)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants