-
Notifications
You must be signed in to change notification settings - Fork 234
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] - refactor(Data/Matrix/Basic): use a defeq for scalar that matches its docstring #8115
Conversation
…ast` This also gives the `intCast` instance a nicer defeq. This remains unopinionated about which side of `diagonal 37 = 37` is the simp-normal form.
This PR/issue depends on: |
#noalign matrix.scalar_apply_eq | ||
#noalign matrix.scalar_apply_ne |
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 would prefer to keep these two specialized apply
lemmas (they don't need to be @[simp]
), it seems to save a bit of work down the line. Also because many of the specializations of diagonal
also have this apply_eq
and apply_ne
variation.
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.
Also because many of the specializations of
diagonal
also have thisapply_eq
andapply_ne
variation.
What lemmas are you thinking of?
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 quick search for apply_(of_)?ne
gives the following @[simp]
lemmas about on/off diagonal entries of matrices: one_apply_{eq,ne}
, charmatrix_apply_{eq,ne}
, stdBasisMatrix.apply_{same,of_ne}
, stdBasisMatrix.mul_{left,right}_apply_of_ne
, transvection_mul_apply_of_ne
.
More generally, we have AffineBasis.coord_apply_{eq,ne}
, CategoryTheory.Mat.id_apply_of_ne
, Finset.affineCombinationSingleWeights_apply_of_ne
, Finset.weightedVSubVSubWeights_apply_of_ne
, Finset.affineCombinationLineMapWeights_apply_of_ne
, Finsupp.single_eq_of_ne
.
I haven't counted but these apply lemmas are more commonly marked as @[simp]
than not.
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'd argue it's better to just force the user to unfold scalar
(using scalar_apply
) if they want any lemmas about it; its primary purpose is to exist as a bundled morphism. The alternative feels like a path to duplicating all the diagonal
API for scalar
, which seems like a lot of work to save using a dsimp lemma.
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.
Alright! I haven't been entirely convinced but it's not worth holding up this PR on that.
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.
Only the question about keeping the apply
lemmas, LGTM otherwise!
#noalign matrix.scalar_apply_eq | ||
#noalign matrix.scalar_apply_ne |
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 quick search for apply_(of_)?ne
gives the following @[simp]
lemmas about on/off diagonal entries of matrices: one_apply_{eq,ne}
, charmatrix_apply_{eq,ne}
, stdBasisMatrix.apply_{same,of_ne}
, stdBasisMatrix.mul_{left,right}_apply_of_ne
, transvection_mul_apply_of_ne
.
More generally, we have AffineBasis.coord_apply_{eq,ne}
, CategoryTheory.Mat.id_apply_of_ne
, Finset.affineCombinationSingleWeights_apply_of_ne
, Finset.weightedVSubVSubWeights_apply_of_ne
, Finset.affineCombinationLineMapWeights_apply_of_ne
, Finsupp.single_eq_of_ne
.
I haven't counted but these apply lemmas are more commonly marked as @[simp]
than not.
bors merge |
…docstring (#8115) This changes the defeq from `scalar a = a • 1` to `scalar a = diagonal fun _ => a`, which has the nice bonus of making `algebraMap_eq_diagonal` true by `rfl`. As a result, we need a new `smul_one_eq_diagonal` lemma to rewrite `diagonal fun _ => a` back into `a • 1`, along with some variants for convenience. In the long term we could generalize this to non-unital rings, now that it needs no `1`.
Pull request successfully merged into master. Build succeeded: |
…docstring (#8115) This changes the defeq from `scalar a = a • 1` to `scalar a = diagonal fun _ => a`, which has the nice bonus of making `algebraMap_eq_diagonal` true by `rfl`. As a result, we need a new `smul_one_eq_diagonal` lemma to rewrite `diagonal fun _ => a` back into `a • 1`, along with some variants for convenience. In the long term we could generalize this to non-unital rings, now that it needs no `1`.
This changes the defeq from
scalar a = a • 1
toscalar a = diagonal fun _ => a
, which has the nice bonus of makingalgebraMap_eq_diagonal
true byrfl
.As a result, we need a new
smul_one_eq_diagonal
lemma to rewritediagonal fun _ => a
back intoa • 1
, along with some variants for convenience.In the long term we could generalize this to non-unital rings, now that it needs no
1
.natCast
andintCast
#8088