[Merged by Bors] - refactor(LinearAlgebra) : Remove unused commutativity hypothesis#9475
Closed
[Merged by Bors] - refactor(LinearAlgebra) : Remove unused commutativity hypothesis#9475
Conversation
2 tasks
Contributor
If you don't want to do this, can you add a comment to this effect? maintainer merge |
|
🚀 Pull request has been placed on the maintainer queue by ericrbg. |
eric-wieser
reviewed
Jan 6, 2024
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Collaborator
Author
Done. |
Member
|
!bench |
Collaborator
|
Here are the benchmark results for commit ab9623c. |
Member
|
bors merge |
mathlib-bors bot
pushed a commit
that referenced
this pull request
Jan 6, 2024
Remove unused commutativity hypothesis: * Removes the requirement for the Semirings to be commutative in `LinearMap.ext_basis` and `LinearMap.sum_repr_mul_repr_mulₛₗ` in `LinearAlgebra/Basis/Bilinear` * Remove the requirement for some Semirings to be commutative in `AuxToLinearMap`, `CommSemiring` and `CommRing` in `LinearAlgebra/Matrix/SesquilinearForm` * In addition, the rings in `CommRing` can just be `Semiring` No changes to the proofs are required. It would also be possible to weaken commutativity from `Rₗ` in `LinearMap.sum_repr_mul_repr_mul` to `[SMulCommClass Rₗ Rₗ Pₗ]` in order to make `sum_repr_mul_repr_mulₛₗ` and `sum_repr_mul_repr_mul` consistent, but I have not done that in this PR because there might be a performance impact (see #7538 (review)). Co-authored-by: Christopher Hoskin <mans0954@users.noreply.github.com> Co-authored-by: Christopher Hoskin <christopher.hoskin@overleaf.com>
Contributor
|
Pull request successfully merged into master. Build succeeded: |
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 <mans0954@users.noreply.github.com> Co-authored-by: Christopher Hoskin <christopher.hoskin@overleaf.com>
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
5 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.
Remove unused commutativity hypothesis:
LinearMap.ext_basisandLinearMap.sum_repr_mul_repr_mulₛₗinLinearAlgebra/Basis/BilinearAuxToLinearMap,CommSemiringandCommRinginLinearAlgebra/Matrix/SesquilinearFormCommRingcan just beSemiringNo changes to the proofs are required.
It would also be possible to weaken commutativity from
RₗinLinearMap.sum_repr_mul_repr_multo[SMulCommClass Rₗ Rₗ Pₗ]in order to makesum_repr_mul_repr_mulₛₗandsum_repr_mul_repr_mulconsistent, but I have not done that in this PR because there might be a performance impact (see #7538 (review)).Used in #9334