-
Notifications
You must be signed in to change notification settings - Fork 297
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(linear_algebra/matrix/reindex): add some lemmas #7963
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few stylistic suggestions:
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
For the time being, it does not seems to me that the
but can't understand what to fix. That being said, I have two problems with
|
Add the file to the correct place.
src/data/matrix/basic.lean
Outdated
@@ -964,6 +964,9 @@ lemma transpose_reindex (eₘ : m ≃ l) (eₙ : n ≃ o) (M : matrix m n α) : | |||
(reindex eₘ eₙ M)ᵀ = (reindex eₙ eₘ Mᵀ) := | |||
rfl | |||
|
|||
@[simp] lemma reindex_one [semiring α] [decidable_eq m] [decidable_eq l] (eₘ : m ≃ l) : | |||
(reindex eₘ eₘ (1 : matrix m m α)) = 1 := by simp | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have at any rate wiped off the reindex_one
, as the minor
could close the goal I was interested in, in linear_algebra/matrix/reindex.lean
.
You can see style errors in the "changes" tab of the PR - GitHub tries to be smart and parse out the data, hiding it in the process. I already added a suggestion to fix the long line, so don't worry about this anymore.
I also know
The trick is that |
Ok, all in all I am a bit confused. What should I do to get the |
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
I understand the remarks as referring to the following two guidelines: If the proof of your lemma is simply There's another guideline that says: if you add a lemma where The result for the |
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks for perservering! I'll let @eric-wieser make the final decision.
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the persistence!
bors d+
end | ||
|
||
lemma mul_one_minor [semiring α] [decidable_eq o] (e₁ : l → o) (e₂ : m ≃ o) (M : matrix m n α) : | ||
((1 : matrix o o α).minor e₁ e₂).mul M = minor M (e₂.symm ∘ e₁) id := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May as well use the notation:
((1 : matrix o o α).minor e₁ e₂).mul M = minor M (e₂.symm ∘ e₁) id := | |
(1 : matrix o o α).minor e₁ e₂ ⬝ M = minor M (e₂.symm ∘ e₁) id := |
✌️ faenuccio can now approve this pull request. To approve and merge a pull request, simply reply with |
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
bors r+ |
From LTE Added lemmas `reindex_linear_equiv_trans`, `reindex_linear_equiv_comp`, `reindex_linear_equiv_comp_apply`, `reindex_linear_equiv_one` and `mul_reindex_linear_equiv_mul_one` needed in LTE. Co-authored-by: Filippo Nuccio <nuccio@cluster18-math.univ-lyon1.fr> Co-authored-by: Filippo A. E. Nuccio <65080144+faenuccio@users.noreply.github.com> Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Pull request successfully merged into master. Build succeeded: |
From LTE
Added lemmas
reindex_linear_equiv_trans
,reindex_linear_equiv_comp
,reindex_linear_equiv_comp_apply
,reindex_linear_equiv_one
andmul_reindex_linear_equiv_mul_one
needed in LTE.