[Merged by Bors] - refactor(LinearAlgebra/Matrix/BilinearForm): Derive BilinearForm results from SesquilinearForm#9485
Closed
[Merged by Bors] - refactor(LinearAlgebra/Matrix/BilinearForm): Derive BilinearForm results from SesquilinearForm#9485
Conversation
Co-authored-by: Eric Wieser <[email protected]>
mans0954
force-pushed
the
mans0954/matrix-bilin-refactor
branch
from
5725d00 to
a652e30
Compare
Collaborator
Author
|
!bench |
Collaborator
|
Here are the benchmark results for commit a652e30. Benchmark Metric Change
======================================================================
- ~Mathlib.LinearAlgebra.Matrix.BilinearForm instructions 22.9%
+ ~Mathlib.LinearAlgebra.Matrix.SesquilinearForm instructions -9.5% |
Member
|
The diff looks really good! But these benchmark results are a bit surprising... @mattrobball do you have a clue what's going on? |
Contributor
|
Were there timeouts in this area of the library? Why all the |
Contributor
|
Performance is fixed by @Vierkantor with leanprover-community/mathlib@Vierkantor/unbundled-FunLike-testings Actually greatly improved :) |
mattrobball
approved these changes
Jan 8, 2024
Contributor
mattrobball
left a comment
mattrobball
left a comment
There was a problem hiding this comment.
I think merging in advance of the pending FunLike refactor is a good plan.
ghost
added
ready-to-merge
mathlib-bors bot
pushed a commit
that referenced
this pull request
Jan 8, 2024
…lts from SesquilinearForm (#9485) Give definitions in `LinearAlgebra/Matrix/BilinearForm` in terms of the equivalent definitions in `LinearAlgebra/Matrix/SesquilinearForm` and derive the `BilinearForm` results as effectively special cases of the equivalent results in `SesquilinearForm`. This reduces the length of `LinearAlgebra/Matrix/BilinearForm` by over 100 lines. The aim is to: * Clarify how results in `BilinearForm` relate to results in `SesquilinearForm` * Reduce duplication of argument between the two files * Validate that the results in `SesquilinearForm` are sufficiently general to provide the results in `BilinearForm` in their existing form - in fact, some loosening of the hypothesis in `SesquilinearForm` is required. Further loosening was already applied in #9475 Co-authored-by: Christopher Hoskin <[email protected]> Co-authored-by: Christopher Hoskin <[email protected]>
Contributor
|
Pull request successfully merged into master. Build succeeded: |
mathlib-bors
bot
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Give definitions in
LinearAlgebra/Matrix/BilinearFormin terms of the equivalent definitions inLinearAlgebra/Matrix/SesquilinearFormand derive theBilinearFormresults as effectively special cases of the equivalent results inSesquilinearForm. This reduces the length ofLinearAlgebra/Matrix/BilinearFormby over 100 lines.The aim is to:
BilinearFormrelate to results inSesquilinearFormSesquilinearFormare sufficiently general to provide the results inBilinearFormin their existing form - in fact, some loosening of the hypothesis inSesquilinearFormis required. Further loosening was already applied in [Merged by Bors] - refactor(LinearAlgebra) : Remove unused commutativity hypothesis #9475This PR partially superceeds #8256