Skip to content

[Merged by Bors] - perf: improve instances in TensorProduct#7401

Closed
ChrisHughes24 wants to merge 3 commits intomasterfrom
TensorProductPerfCH
Closed

[Merged by Bors] - perf: improve instances in TensorProduct#7401
ChrisHughes24 wants to merge 3 commits intomasterfrom
TensorProductPerfCH

Conversation

@ChrisHughes24
Copy link
Copy Markdown
Member


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 27, 2023
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@ChrisHughes24
Copy link
Copy Markdown
Member Author

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 1a787a0.
There were significant changes against commit aa7c8c3:

  Benchmark                                                       Metric         Change
  =====================================================================================
+ ~Mathlib.Algebra.Category.FGModuleCat.Basic                     instructions   -24.5%
+ ~Mathlib.Algebra.Category.ModuleCat.ChangeOfRings               instructions    -7.3%
+ ~Mathlib.Algebra.Category.ModuleCat.Monoidal.Basic              instructions   -54.9%
+ ~Mathlib.Algebra.Category.ModuleCat.Monoidal.Closed             instructions   -23.5%
+ ~Mathlib.CategoryTheory.Monoidal.Internal.Module                instructions    -6.7%
+ ~Mathlib.LinearAlgebra.CliffordAlgebra.BaseChange               instructions    -8.9%
+ ~Mathlib.LinearAlgebra.QuadraticForm.TensorProduct.Isometries   instructions    -7.5%
+ ~Mathlib.LinearAlgebra.TensorPower                              instructions    -6.2%
+ ~Mathlib.LinearAlgebra.TensorProduct.Opposite                   instructions    -6.3%
+ ~Mathlib.LinearAlgebra.TensorProduct.Tower                      instructions    -9.4%
+ ~Mathlib.RepresentationTheory.Character                         instructions    -5.8%
+ ~Mathlib.RepresentationTheory.FdRep                             instructions   -26.2%
+ ~Mathlib.RepresentationTheory.GroupCohomology.Resolution        instructions    -7.8%
+ ~Mathlib.RepresentationTheory.Rep                               instructions   -20.6%
+ ~Mathlib.RingTheory.Kaehler                                     instructions    -5.6%
+ ~Mathlib.RingTheory.MatrixAlgebra                               instructions   -20.2%
+ ~Mathlib.RingTheory.TensorProduct                               instructions    -8.9%

@ChrisHughes24 ChrisHughes24 added awaiting-review and removed WIP Work in progress awaiting-author A reviewer has asked the author a question or requested changes. labels Sep 27, 2023
@ChrisHughes24 ChrisHughes24 changed the title experiment with improving tensor product performance perf: improve instances in TensorProduct Sep 27, 2023
Copy link
Copy Markdown
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

Impressive work!

bors d+

instance addZeroClass : AddZeroClass (M ⊗[R] N) :=
{ (addConGen (TensorProduct.Eqv R M N)).addMonoid with }
{ (addConGen (TensorProduct.Eqv R M N)).addMonoid with
toAdd := TensorProduct.add _ _ }
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.

Could you (in a comment) explain why making the addition explicit is needed? I suppose it's because we want to have the actual ancestor of the class, so unification is faster.

Idem below.

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.

In fact, there should probably be a library note explaining the optimal way to do this kind of inheritance, since it changed quite a bit since Lean 3.

@bors
Copy link
Copy Markdown

bors bot commented Sep 28, 2023

✌️ ChrisHughes24 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 Sep 28, 2023
@ChrisHughes24
Copy link
Copy Markdown
Member Author

bors r+

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Sep 28, 2023
bors bot pushed a commit that referenced this pull request Sep 28, 2023
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
@bors
Copy link
Copy Markdown

bors bot commented Sep 28, 2023

This PR was included in a batch that was canceled, it will be automatically retried

bors bot pushed a commit that referenced this pull request Sep 28, 2023
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
@bors
Copy link
Copy Markdown

bors bot commented Sep 28, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 28, 2023
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
@bors
Copy link
Copy Markdown

bors bot commented Sep 28, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 28, 2023
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
@bors
Copy link
Copy Markdown

bors bot commented Sep 28, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title perf: improve instances in TensorProduct [Merged by Bors] - perf: improve instances in TensorProduct Sep 28, 2023
@bors bors bot closed this Sep 28, 2023
@bors bors bot deleted the TensorProductPerfCH branch September 28, 2023 18:17
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants