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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - chore: bump to nightly-2023-08-17 #6019

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - chore: bump to nightly-2023-08-17 #6019

wants to merge 4 commits into from

Conversation

kim-em
Copy link
Contributor

@kim-em kim-em commented Jul 20, 2023 •
edited by eric-wieser
Loading

@kim-em kim-em added blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) awaiting-review blocked-by-core-PR This PR depends on a PR to Core/Std awaiting-CI labels Jul 20, 2023
@kim-em kim-em mentioned this pull request Jul 20, 2023
Merged
1 task
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) label Jul 20, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jul 27, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 5, 2023
This was referenced Aug 6, 2023
Merged
Closed
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Aug 7, 2023
@kim-em kim-em changed the title chore: adapt to simp failing if it makes no progress chore: bump to nightly-2023-08-17 Aug 17, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 17, 2023
@kim-em kim-em mentioned this pull request Aug 17, 2023
Closed
1 task
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 17, 2023
joneugster
joneugster reviewed Aug 17, 2023
View reviewed changes
Copy link
Collaborator

@joneugster joneugster left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked through all the files, basically everything is just removing superfluous simp-calls, except one deletion marked below.

In my optinion any_goals simp would be better than _ <;> (try simp) or all_goals (try simp) and it's a bit a shame that <;> simp does not work like any_goals simp.

One thing I did not check is if this PR really modifies all tactics that rely on simp making no progress.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 17, 2023
@joneugster joneugster mentioned this pull request Aug 17, 2023
Closed
@kim-em kim-em removed the blocked-by-core-PR This PR depends on a PR to Core/Std label Aug 17, 2023
@kim-em kim-em force-pushed the simp_failIfUnchanged branch from 9a539d8 to 6b5c3f9 Compare August 18, 2023 01:37
eric-wieser
eric-wieser reviewed Aug 18, 2023
View reviewed changes
Mathlib/Tactic/CategoryTheory/BicategoryCoherence.lean Outdated
@@ -295,7 +295,8 @@ syntax (name := whisker_simps) "whisker_simps" : tactic
elab_rules : tactic
| `(tactic| whisker_simps) => do
evalTactic (← `(tactic|
simp only [Category.assoc, Bicategory.comp_whiskerLeft, Bicategory.id_whiskerLeft,
simp (config := {failIfUnchanged := false}) only [Category.assoc,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an explanation of why failIfUnchanged is helpful here? Should whisker_simps take it's own config argument to allow the caller to customize this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It hadn't even occurred to me this counted as a user facing tactic. Mostly it is just used as a component in coherence. But sure, I'll give it a simp configuration.

eric-wieser
eric-wieser approved these changes Aug 18, 2023
View reviewed changes
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors d+

@bors
Copy link

bors bot commented Aug 18, 2023

✌️ semorrison can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@kim-em
Copy link
Contributor Author

kim-em commented Aug 18, 2023

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Aug 18, 2023
bors bot pushed a commit that referenced this pull request Aug 18, 2023
@kim-em @eric-wieser
chore: bump to nightly-2023-08-17 (#6019)
8c53b71 
The major change here is adapting to `simp` failing if it makes no progress.
The vast majority of the redundant `simp`s found due to this change were extracted to #6632.



Co-authored-by: Scott Morrison <[email protected]>
Co-authored-by: Eric Wieser <[email protected]>
@bors
Copy link

bors bot commented Aug 18, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title chore: bump to nightly-2023-08-17 [Merged by Bors] - chore: bump to nightly-2023-08-17 Aug 18, 2023
@bors bors bot closed this Aug 18, 2023
@bors bors bot deleted the simp_failIfUnchanged branch August 18, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser approved these changes

@joneugster joneugster joneugster left review comments

Assignees
No one assigned
Labels
delegated ready-to-merge This PR has been sent to bors.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kim-em @eric-wieser @joneugster @leanprover-community-mathlib4-bot