Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: heterogenize TensorProduct.congr and friends #6035
[Merged by Bors] - feat: heterogenize TensorProduct.congr and friends #6035
Changes from all commits
02508fd
187f03f
30b1cbe
7ef1780
4264b66
f328795
dd7166c
52df98e
d68de54
5f3c6df
e4ab5b6
dfaf5ad
7212072
9686573
f7b026f
4dbcc44
0b43277
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is this a candidate for
@[simps]
? I'm only just getting the hang of@[simps]
. Or is the idea that you're safer writing your own simp lemmas for declarations as complicated as this?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.
simps
generates something with implicit type arguments, which aren't really needed here; though I'll admit I don't remember if that was my reason.In generaly my rule is "use
simps
if I'm feeling lazy, write the lemma manually if I'm feeling picky or simps screws up". The important thing is to have somesimp
lemma, the API doesn't really care whether it's generated withsimps
.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.
It genuinely does my head in that you don't need
IsScalarTower R A B
here. In the kind of mathematics I'm used to, I'm not sure it would ever happen that A and B are R-algebras and B is an A-algebra and the diagram doesn't commute. However I am not used to non-commutative ring theory and I see that B isn't commutative.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 started to reach the point where my only real criteria for "should I add this typeclass" was simultaneously satisfying:
(M ⊗[A] P) ⊗[R] Q ≃ₗ[A] M ⊗[A] (P ⊗[R] Q)
works(M ⊗[R] P) ⊗[R] Q ≃ₗ[A] M ⊗[R] (P ⊗[R] Q)
also worksI think part of the reason this is so confusing is that once you pile enough scalar tower classes on, they start implying other scalar tower classes (at least, mathematically; most of them aren't safe as
instance
s)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.
Do you think there's a version of this where
[IsScalarTower A B M]
is replaced by[IsScalarTower A B P]
? Note that I'm not suggesting that you add it, but if you think that there's some merit to this suggestion then you could leave a comment. This seems to be the only def where you need to assume B is an A-algebra.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 don't know. This was one where the "obvious" implementation required more typeclasses than the statement. I think I recall places in mathlib where the "clever" bundling approach requires more typeclasses than the dumb but annoying
induction
approach, but I was pretty maximally annoyed by this point and didn't want to think much more about generalization!