Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(data/matrix,linear_algebra): Use matrix.mul as default multiplication in matrix lemmas #1959

Merged

Conversation

Vierkantor
Copy link
Collaborator

As discussed on Zulip, some simp lemmas for matrices use has_mul.mul, others (the vast majority, it turns out when making this PR) use matrix.mul. Since we agreed that rewriting from matrix.mul to has_mul.mul can cause problems, I decided to be bold and make this PR.

This PR standardises the lemmas for matrices to use matrix.mul. It also standardises the naming where there is a pair of lemmas: foo uses matrix.mul and foo' uses has_mul.mul. Both changes make a few exceptions fit in with the majority of existing definitions.

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

For reviewers: code review check list

Vierkantor added a commit to Vierkantor/mathlib that referenced this pull request Feb 5, 2020
That PR changes `det_mul` to have another, still definitionally equal, type.
If the invocations to `det_mul` are independent of syntactic equality, i.e. we
only pass `det_mul` to `erw`, this branch should be compatible with the state
before the change and after.
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

I'm fine with making these changes. But I'll leave some time for others to look at this PR.

@robertylewis robertylewis added the awaiting-review The author would like community review of the PR label Feb 5, 2020
mergify bot added a commit that referenced this pull request Feb 8, 2020
* Define the special linear group

* Make definitions independent of PR #1959

That PR changes `det_mul` to have another, still definitionally equal, type.
If the invocations to `det_mul` are independent of syntactic equality, i.e. we
only pass `det_mul` to `erw`, this branch should be compatible with the state
before the change and after.

* Documentation and code style improvements

* Improve module docstring

* Fix documentation

`matrix.special_linear_group` is not a set but a type

Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>

* Don't directly coerce from SL to linear maps

Now we coerce from `matrix.special_linear_group n R` to `matrix n n R` instead
of `general_linear_group R (n -> R)`

* Whitespace fixes

* Fix failing build in `src/linear_algebra/dual.lean`

* Give an almost generic formula for `det_adjugate`

* Move `det_eq_one_of_card_eq_zero` to the correct section

* Replace the ad-hoc assumption of `det_adjugate_of_invertible` with `is_unit`

* Fix linting error

There was an unnecessary assumption [decidable_eq α] floating around

* Replace `special_linear_group.val` with the appropriate coercions

* whitespace

Correctly indent continued line

Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>

* Docstrings for the `det_adjugate_of_...` lemmas

Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@sgouezel sgouezel added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Feb 8, 2020
@mergify mergify bot merged commit 777f214 into leanprover-community:master Feb 9, 2020
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…ommunity#1960)

* Define the special linear group

* Make definitions independent of PR leanprover-community#1959

That PR changes `det_mul` to have another, still definitionally equal, type.
If the invocations to `det_mul` are independent of syntactic equality, i.e. we
only pass `det_mul` to `erw`, this branch should be compatible with the state
before the change and after.

* Documentation and code style improvements

* Improve module docstring

* Fix documentation

`matrix.special_linear_group` is not a set but a type

Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>

* Don't directly coerce from SL to linear maps

Now we coerce from `matrix.special_linear_group n R` to `matrix n n R` instead
of `general_linear_group R (n -> R)`

* Whitespace fixes

* Fix failing build in `src/linear_algebra/dual.lean`

* Give an almost generic formula for `det_adjugate`

* Move `det_eq_one_of_card_eq_zero` to the correct section

* Replace the ad-hoc assumption of `det_adjugate_of_invertible` with `is_unit`

* Fix linting error

There was an unnecessary assumption [decidable_eq α] floating around

* Replace `special_linear_group.val` with the appropriate coercions

* whitespace

Correctly indent continued line

Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>

* Docstrings for the `det_adjugate_of_...` lemmas

Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…tiplication in matrix lemmas (leanprover-community#1959)

* Change `has_mul.mul` to `matrix.mul` in a few `simp` lemmas

* Standardise more lemmas for matrix multiplication

* Generalize `to_pequiv_mul_matrix` to rectangular matrices

Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…ommunity#1960)

* Define the special linear group

* Make definitions independent of PR leanprover-community#1959

That PR changes `det_mul` to have another, still definitionally equal, type.
If the invocations to `det_mul` are independent of syntactic equality, i.e. we
only pass `det_mul` to `erw`, this branch should be compatible with the state
before the change and after.

* Documentation and code style improvements

* Improve module docstring

* Fix documentation

`matrix.special_linear_group` is not a set but a type

Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>

* Don't directly coerce from SL to linear maps

Now we coerce from `matrix.special_linear_group n R` to `matrix n n R` instead
of `general_linear_group R (n -> R)`

* Whitespace fixes

* Fix failing build in `src/linear_algebra/dual.lean`

* Give an almost generic formula for `det_adjugate`

* Move `det_eq_one_of_card_eq_zero` to the correct section

* Replace the ad-hoc assumption of `det_adjugate_of_invertible` with `is_unit`

* Fix linting error

There was an unnecessary assumption [decidable_eq α] floating around

* Replace `special_linear_group.val` with the appropriate coercions

* whitespace

Correctly indent continued line

Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>

* Docstrings for the `det_adjugate_of_...` lemmas

Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…tiplication in matrix lemmas (leanprover-community#1959)

* Change `has_mul.mul` to `matrix.mul` in a few `simp` lemmas

* Standardise more lemmas for matrix multiplication

* Generalize `to_pequiv_mul_matrix` to rectangular matrices

Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants