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

[Merged by Bors] - feat: make rank_mul_eq_left_of_isUnit_det into an effective simp lemma #12244

Closed
wants to merge 15 commits into from

Conversation

JonBannon
Copy link
Collaborator

@JonBannon JonBannon commented Apr 18, 2024

These theorems give that if A is a unit, or a unitary, in the n x n matrices over a commutative ring R, and B is an m x n matrix over R, that multiplying B by A on the left or right doesn't change the rank of B. To facilitate this, added isUnits_det_units and det_isUnit lemmas.


Open in Gitpod

@github-actions github-actions bot added the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 18, 2024
@JonBannon JonBannon changed the title feat: add Theorems rank_mul_units and rank_units_mul feat: add Theorems rank_mul_units, rank_units_mul, rank_mul_unitary and rank_unitary_mul Apr 18, 2024
@JonBannon JonBannon added the awaiting-review The author would like community review of the PR label Apr 18, 2024
Mathlib/Data/Matrix/Rank.lean Outdated Show resolved Hide resolved
Mathlib/LinearAlgebra/Matrix/Spectrum.lean Outdated Show resolved Hide resolved
@j-loreaux j-loreaux changed the title feat: add Theorems rank_mul_units, rank_units_mul, rank_mul_unitary and rank_unitary_mul feat: add theorems rank_mul_units, rank_units_mul, rank_mul_unitary and rank_unitary_mul Apr 18, 2024
@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Apr 18, 2024
…tion, putting the [DecidableEq n] into each theorem statement for rank_units_mul and rank_mul_units
@JonBannon JonBannon added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Apr 18, 2024
@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Apr 18, 2024
Copy link
Member

@eric-wieser eric-wieser 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 a little sad that this API has to exist. The problem seems to be that we don't have a standard way to talk about whether a matrix is invertible, and so we end up writing lemmas for every possible way we might want to write it (IsUnit M, IsUnit M.det, M : Units (Matrix _ _ _), Invertible M).

If we declared IsUnit M the simp-normal form of IsUnit M.det (or possibly vice-versa), then we could mark Units.isUnit simp and we probably wouldn't need these new lemmas.

@j-loreaux
Copy link
Collaborator

How would simp actually apply the IsUnit version? I didn't think this problem is specific to matrices at all.

JonBannon and others added 4 commits April 19, 2024 16:50
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Good to know!

Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
@JonBannon JonBannon added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Apr 19, 2024
@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Apr 21, 2024
@JonBannon JonBannon added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Apr 21, 2024
@j-loreaux j-loreaux force-pushed the rank_mul_units-and-rank_units_mul branch from 8bae758 to efb7241 Compare April 21, 2024 15:01
@eric-wieser
Copy link
Member

How would simp actually apply the IsUnit version?

simp would automatically solve any side-goals of the form IsUnit (Units.val u), since the unit-ness is structural.

@j-loreaux
Copy link
Collaborator

j-loreaux commented Apr 22, 2024

@eric-wieser ha! I didn't know that worked. This wasn't available in Lean 3, was it?

I think in the Matrix library we phrase things in terms of IsUnit (det A) instead of IsUnit A almost all the time. At the same time, I don't think we actually want Matrix.isUnit_iff_isUnit_det to be a simp lemma.

One thing we could do: mark rank_mul_eq_right_of_isUnit_det and similar as @[simp], and then add simp lemmas, which then allow these to trigger.

import Mathlib

-- some hypotheses too strong maybe
variable {m n R : Type*} [Fintype n] [CommRing R] [StarRing R] [DecidableEq n]
namespace Matrix

@[simp]
lemma isUnit_det_units (A : (Matrix n n R)ˣ) : IsUnit (det (A : Matrix n n R)) :=
  A.isUnit.map detMonoidHom

@[simp]
lemma isUnit_det_unitaryGroup (A : unitaryGroup n R) : IsUnit (det (A : Matrix n n R)) :=
  (unitary.toUnits A).isUnit.map detMonoidHom

attribute [simp] rank_mul_eq_left_of_isUnit_det rank_mul_eq_right_of_isUnit_det
  isUnit_det_of_invertible

-- then the following work `by simp`

theorem rank_mul_unitary (A : unitaryGroup n R) (B : Matrix m n R) :
    rank (B * (A : Matrix n n R)) = rank B := by
  simp? -- simp only [isUnit_det_unitaryGroup, rank_mul_eq_left_of_isUnit_det]

theorem rank_unitary_mul [Fintype m] (A : unitaryGroup n R)
    (B : Matrix n m R) : rank ((A : Matrix n n R) * B) = rank B := by
  simp? -- simp only [isUnit_det_unitaryGroup, rank_mul_eq_right_of_isUnit_det]

end Matrix

@j-loreaux
Copy link
Collaborator

j-loreaux commented Apr 24, 2024

@eric-wieser I've implemented the change I posted above. Even if we decide that we want to refactor the matrix library and how we handle invertibility, it shouldn't be part of this PR. But now I've rewritten the content of this PR, so I shouldn't merge it.

@j-loreaux j-loreaux changed the title feat: add theorems rank_mul_units, rank_units_mul, rank_mul_unitary and rank_unitary_mul feat: make rank_mul_eq_left_of_isUnit_det into an effective simp lemma Apr 24, 2024
@eric-wieser
Copy link
Member

!bench

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good now. Let's check there's no major performance regression.

Can you check the PR description is up to date with the new version?

bors d+

@mathlib-bors
Copy link

mathlib-bors bot commented May 1, 2024

✌️ JonBannon can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added delegated and removed awaiting-review The author would like community review of the PR labels May 1, 2024
@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit a3f60b1.
There were no significant changes against commit e8433a6.

@JonBannon
Copy link
Collaborator Author

bors r+

mathlib-bors bot pushed a commit that referenced this pull request May 1, 2024
…lemma (#12244)

These theorems give that if A is a unit, or a unitary, in the n x n matrices over a commutative ring R, and B is an m x n matrix over R, that multiplying B by A on the left or right doesn't change the rank of B. To facilitate this, added `isUnits_det_units` and `det_isUnit` lemmas.



Co-authored-by: JonBannon <59937998+JonBannon@users.noreply.github.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
@mathlib-bors
Copy link

mathlib-bors bot commented May 1, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: make rank_mul_eq_left_of_isUnit_det into an effective simp lemma [Merged by Bors] - feat: make rank_mul_eq_left_of_isUnit_det into an effective simp lemma May 1, 2024
@mathlib-bors mathlib-bors bot closed this May 1, 2024
@mathlib-bors mathlib-bors bot deleted the rank_mul_units-and-rank_units_mul branch May 1, 2024 21:44
apnelson1 pushed a commit that referenced this pull request May 12, 2024
…lemma (#12244)

These theorems give that if A is a unit, or a unitary, in the n x n matrices over a commutative ring R, and B is an m x n matrix over R, that multiplying B by A on the left or right doesn't change the rank of B. To facilitate this, added `isUnits_det_units` and `det_isUnit` lemmas.



Co-authored-by: JonBannon <59937998+JonBannon@users.noreply.github.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
callesonne pushed a commit that referenced this pull request May 16, 2024
…lemma (#12244)

These theorems give that if A is a unit, or a unitary, in the n x n matrices over a commutative ring R, and B is an m x n matrix over R, that multiplying B by A on the left or right doesn't change the rank of B. To facilitate this, added `isUnits_det_units` and `det_isUnit` lemmas.



Co-authored-by: JonBannon <59937998+JonBannon@users.noreply.github.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants