[Merged by Bors] - perf(library/congr_lemma): do not check for subsingleton in simp congrs #665
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hmmm, this still synthesizes subsingleton instances in simp. ?!?!? |
Vierkantor
reviewed
Jan 10, 2022
|
bors merge |
bors bot
pushed a commit
that referenced
this pull request
Jan 11, 2022
…rs (#665) This changes the automatically-generated simp congr lemma in several situations: - Subsingleton (non-prop) instance arguments are Fixed instead of Cast. Which is very good, because Cast introduces diamonds. - However, decidable instances (decidable/decidable_pred/decidable_rel) have a new special kind called SubsingletonInst. The automatically generated lemma looks exactly like the old `if_congr` (which is no longer marked congr). - If you have arguments like `(a : α) (b : β a)` where `b` is a subsingleton, then simp can no longer rewrite in `a`. Not sure if this actually pops up anywhere in practice (for decidability arguments see above). This also affects the `congr` tactic slightly since the specialization prefix size is computed without taking subsingleton information into account.
|
Pull request successfully merged into master. Build succeeded: |
bors
bot
changed the title
bors bot
pushed a commit
to leanprover-community/mathlib
that referenced
this pull request
Aug 15, 2022
) + Add `unique` instances when the codomain types are subsingletons and rename the original `unique` instances (which apply when the domain is empty) to `unique_of_is_empty`. These are in analogy to [pi.unique](https://leanprover-community.github.io/mathlib_docs/logic/unique.html#pi.unique) and [pi.unique_of_is_empty](https://leanprover-community.github.io/mathlib_docs/logic/unique.html#pi.unique_of_is_empty). + Golf the `unique` instances for (d)finsupp using `fun_like.coe_injective.unique`. The names `unique_of_left` and `unique_of_right` remain unchanged in the finsupp case, because finsupp is special in that it consists of non-dependent functions, unlike dfinsupp or direct_sum. (There was a concern that adding `unique` instances would slow down `simp` (see #6025), but it has been fixed in [lean#665](leanprover-community/lean#665).)
bors bot
pushed a commit
to leanprover-community/mathlib
that referenced
this pull request
Aug 15, 2022
#### Background The `simp` tactic used to [look for subsingleton instances excessively](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/223034551), so every subsingleton/unique instances added to mathlib could slow down `simp` performance; in fact, removing some subsingleton instances locally led to such speedup that three hacks #5779, #5878 and #6639 along this line have been merged into mathlib. Since the problem was discovered, new subsingleton instances were mostly turned into lemma/defs, with a reference to gh-6025 that @eric-wieser created to track them and a notice to restore them as instances once safe to do so. This bad behavior of `simp` was finally fixed in [lean#665](leanprover-community/lean#665) by @gebner in January 2022, so it is now safe to remove the hacks and restore the instances. In fact, the hack #5878 was already removed in [#13127](c7626b7#diff-02976f8e19552b3c7ea070e30761e4e99f1333085cc5bb94fa170861c6ed27bcL497). I [measured the difference in performance](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/293228091) from removing one of the remaining hacks and it was negligible. Therefore, in this PR, we: + Remove both hacks remaining; + Turn all would-be instances that mention gh-6025 into actual global instances; + Golf proofs that explicitly invoked these instances previously; + Remove `local attribute [instance]` lines that were added when these instances were needed. Closes #6025.
bors bot
pushed a commit
to leanprover-community/mathlib
that referenced
this pull request
Aug 15, 2022
#### Background The `simp` tactic used to [look for subsingleton instances excessively](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/223034551), so every subsingleton/unique instances added to mathlib could slow down `simp` performance; in fact, removing some subsingleton instances locally led to such speedup that three hacks #5779, #5878 and #6639 along this line have been merged into mathlib. Since the problem was discovered, new subsingleton instances were mostly turned into lemma/defs, with a reference to gh-6025 that @eric-wieser created to track them and a notice to restore them as instances once safe to do so. This bad behavior of `simp` was finally fixed in [lean#665](leanprover-community/lean#665) by @gebner in January 2022, so it is now safe to remove the hacks and restore the instances. In fact, the hack #5878 was already removed in [#13127](c7626b7#diff-02976f8e19552b3c7ea070e30761e4e99f1333085cc5bb94fa170861c6ed27bcL497). I [measured the difference in performance](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/293228091) from removing one of the remaining hacks and it was negligible. Therefore, in this PR, we: + Remove both hacks remaining; + Turn all would-be instances that mention gh-6025 into actual global instances; + Golf proofs that explicitly invoked these instances previously; + Remove `local attribute [instance]` lines that were added when these instances were needed. Closes #6025.
bors bot
pushed a commit
to leanprover-community/mathlib
that referenced
this pull request
Aug 15, 2022
#### Background The `simp` tactic used to [look for subsingleton instances excessively](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/223034551), so every subsingleton/unique instances added to mathlib could slow down `simp` performance; in fact, removing some subsingleton instances locally led to such speedup that three hacks #5779, #5878 and #6639 along this line have been merged into mathlib. Since the problem was discovered, new subsingleton instances were mostly turned into lemma/defs, with a reference to gh-6025 that @eric-wieser created to track them and a notice to restore them as instances once safe to do so. This bad behavior of `simp` was finally fixed in [lean#665](leanprover-community/lean#665) by @gebner in January 2022, so it is now safe to remove the hacks and restore the instances. In fact, the hack #5878 was already removed in [#13127](c7626b7#diff-02976f8e19552b3c7ea070e30761e4e99f1333085cc5bb94fa170861c6ed27bcL497). I [measured the difference in performance](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/293228091) from removing one of the remaining hacks and it was negligible. Therefore, in this PR, we: + Remove both hacks remaining; + Turn all would-be instances that mention gh-6025 into actual global instances; + Golf proofs that explicitly invoked these instances previously; + Remove `local attribute [instance]` lines that were added when these instances were needed. Closes #6025.
bors bot
pushed a commit
to leanprover-community/mathlib
that referenced
this pull request
Aug 16, 2022
#### Background The `simp` tactic used to [look for subsingleton instances excessively](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/223034551), so every subsingleton/unique instances added to mathlib could slow down `simp` performance; in fact, removing some subsingleton instances locally led to such speedup that three hacks #5779, #5878 and #6639 along this line have been merged into mathlib. Since the problem was discovered, new subsingleton instances were mostly turned into lemma/defs, with a reference to gh-6025 that @eric-wieser created to track them and a notice to restore them as instances once safe to do so. This bad behavior of `simp` was finally fixed in [lean#665](leanprover-community/lean#665) by @gebner in January 2022, so it is now safe to remove the hacks and restore the instances. In fact, the hack #5878 was already removed in [#13127](c7626b7#diff-02976f8e19552b3c7ea070e30761e4e99f1333085cc5bb94fa170861c6ed27bcL497). I [measured the difference in performance](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.235672.20breaks.20timeout/near/293228091) from removing one of the remaining hacks and it was negligible. Therefore, in this PR, we: + Remove both hacks remaining; + Turn all would-be instances that mention gh-6025 into actual global instances; + Golf proofs that explicitly invoked these instances previously; + Remove `local attribute [instance]` lines that were added when these instances were needed. Closes #6025.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This changes the automatically-generated simp congr lemma in several situations:
if_congr(which is no longer marked congr).(a : α) (b : β a)wherebis a subsingleton, then simp can no longer rewrite ina. Not sure if this actually pops up anywhere in practice (for decidability arguments see above).This also affects the
congrtactic slightly since the specialization prefix size is computed without taking subsingleton information into account.