Skip to content
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] - fix(normed_space/multilinear): speed up slow proof #6639

Closed
wants to merge 2 commits into from

Conversation

robertylewis
Copy link
Member

This proof seems to be right on the edge of timing out and has been causing CI issues.

I'm not sure if this is the only culprit. This whole file is very slow. Is this because of recent changes, or has it always been like this?


@robertylewis robertylewis added the awaiting-review The author would like community review of the PR label Mar 11, 2021
@urkud
Copy link
Member

urkud commented Mar 11, 2021

LGTM

@gebner
Copy link
Member

gebner commented Mar 11, 2021

bors r+

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Mar 11, 2021
@gebner
Copy link
Member

gebner commented Mar 11, 2021

bors p=1001

@gebner
Copy link
Member

gebner commented Mar 11, 2021

bors r-
bors r+

@bors
Copy link

bors bot commented Mar 11, 2021

Canceled.

bors bot pushed a commit that referenced this pull request Mar 11, 2021
This proof seems to be right on the edge of timing out and has been causing CI issues.

I'm not sure if this is the only culprit. This whole file is very slow. Is this because of recent changes, or has it always been like this?



Co-authored-by: Rob Lewis <rob.y.lewis@gmail.com>
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@gebner
Copy link
Member

gebner commented Mar 11, 2021

Hmm, sorry that was not what the documentation said p=1001 would do.

Set the priority of the current pull request. Pull requests with different priority are never batched together. The pull request with the bigger priority number goes first.

It didn't say anything about cancelling the current batch. And it's also batched together with other PRs.

@bors
Copy link

bors bot commented Mar 11, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix(normed_space/multilinear): speed up slow proof [Merged by Bors] - fix(normed_space/multilinear): speed up slow proof Mar 11, 2021
@bors bors bot closed this Mar 11, 2021
@bors bors bot deleted the multilinear-perf branch March 11, 2021 17:09
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
This proof seems to be right on the edge of timing out and has been causing CI issues.

I'm not sure if this is the only culprit. This whole file is very slow. Is this because of recent changes, or has it always been like this?



Co-authored-by: Rob Lewis <rob.y.lewis@gmail.com>
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
bors bot pushed a commit 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 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 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 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
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants