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/basis) remove several [nontrivial R] #7642
Conversation
src/linear_algebra/basis.lean
Outdated
def reindex_range [nontrivial R] : basis (range b) R M := | ||
b.reindex (equiv.of_injective b b.injective) | ||
def reindex_range : basis (range b) R M := | ||
begin |
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 am not sure it is OK to use tactic mode in a def
.
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.
Hmm, I'm not sure this is a good idea. It would be nice if we can avoid by_cases
in this definition.
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.
Let me see if there is a better proof.
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 think this is fine. The lemmas below show that you can still reason about the value of this definition. You could also use or.by_cases
in term mode, but you will probably end up with the same / a similar term.
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.
Personally, I would prefer if
for definitions:
if h : nontrivial R then
by exactI
...
else
by haveI : subsingleton R := ...; exact
...
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 like the if
! I have the impression we have to reason by cases, in a way or another: the two proofs are really different. Indeed if R
is trivial this is true just because anything is a basis (and the rank is not well defined!).
b.reindex_range ⟨b i, h⟩ = b i := | ||
by rw [reindex_range, reindex_apply, equiv.apply_of_injective_symm b b.injective, subtype.coe_mk] | ||
begin | ||
by_cases htr : nontrivial R, |
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.
You might be able to use the nontriviality
tactic to automatically do the not-nontrivial case for you.
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 otherwise, thank you!
bors d+
✌️ riccardobrasca can now approve this pull request. To approve and merge a pull request, simply reply with |
bors r+ |
Canceled. |
bors r+ |
We remove some unnecessary `nontrivial R`.
Pull request successfully merged into master. Build succeeded: |
We remove some unnecessary `nontrivial R`.
We remove some unnecessary
nontrivial R
.