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] - chore(group_theory/perm/sign): remove classical for sign congr simp lemmas #5622
Conversation
rw [← finset.univ_product_univ, finset.prod_product, finset.prod_comm], | ||
simp }, | ||
rw [finset.prod_mul_distrib, ←finset.univ_product_univ, finset.prod_product, finset.prod_comm], | ||
simp [sign_prod_congr_left] }, |
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.
This is a great improvement!
I wasn't aware that using classical
like this could be an issue. Are there other places where this pops up?
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.
Yes, the sign
lemmas simplifying various congr
were causing me a lot of pain trying to simplify pretty obvious equiv_congr.map_equiv option
, because the decidable_eq
instance for option α
is easily constructed from the [decidable_eq α]
, but the relevant lemma had no explicit decidable_eq
! I was so confused, until I noticed the open_locale classical
that was near the end of the file. Some new sign
lemmas didn't have this issue because they used different Type
arguments.
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 found an earlier Zulip thread that discussed a similar issue and mentioned this PR on Zulip.
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 great, thanks!
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 🎉
bors merge
…emmas (#5622) Previously, some lemmas about how `perm.sign` simplifies across various congrs of permutations assumed `classical`, which prevented them from being applied by the simplifier. This makes the `decidable_eq` assumptions explicit.
Pull request successfully merged into master. Build succeeded: |
Previously, some lemmas about how
perm.sign
simplifies across various congrs of permutations assumedclassical
, which prevented them from being applied by the simplifier. This makes thedecidable_eq
assumptions explicit.