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] - chore: adaptations for nightly-2024-02-29 #11070

Closed
wants to merge 31 commits into from

Conversation

semorrison
Copy link
Contributor

@semorrison semorrison commented Feb 29, 2024

field_simp is a very ambitious tactic, that tries to prove that it can clear denominators. Previously it was taking advantage of essentially a bug in the simp discharging framework, so that parts of its discharging strategy were not limited by maxDischargeDepth (so it could "keep trying"), but everyone else was limited (so simp didn't waste time exploring large trees of discharges). Now the maxDischargeDepth is enforced for everyone, so to keep field_simp working we have to increase its maxDischargeDepth. That makes it extremely slow. I've kept the maxDischargeDepth to a minimum, but still there are some set_option maxHeartbeats (all with notes attached).

After we've merged this, someone is going to have to rewrite field_simp to make it performant.


#11079 is backporting a few changes to master

Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

A few non-expert comments - it seems this could use a merge of mathlib's master branch.

I was wondering: are the remaining Adaptation notes supposed to stay? If so, would a tracking issue for them be useful, so they don't get forgotten?

Mathlib/Algebra/Free.lean Outdated Show resolved Hide resolved
Mathlib/Algebra/Order/Ring/WithTop.lean Outdated Show resolved Hide resolved
Mathlib/Analysis/Normed/Group/Seminorm.lean Show resolved Hide resolved
Mathlib/CategoryTheory/Limits/Shapes/ZeroMorphisms.lean Outdated Show resolved Hide resolved
Mathlib/LinearAlgebra/FreeModule/Norm.lean Outdated Show resolved Hide resolved
Mathlib/Order/Filter/AtTopBot.lean Outdated Show resolved Hide resolved
Mathlib/RingTheory/Polynomial/ScaleRoots.lean Outdated Show resolved Hide resolved
Mathlib/Algebra/Free.lean Outdated Show resolved Hide resolved
Mathlib/Analysis/SpecificLimits/Normed.lean Show resolved Hide resolved
Mathlib/Data/Nat/Digits.lean Outdated Show resolved Hide resolved
Mathlib/Data/Real/EReal.lean Outdated Show resolved Hide resolved
Mathlib/MeasureTheory/Integral/SetToL1.lean Outdated Show resolved Hide resolved
Mathlib/Order/BooleanAlgebra.lean Outdated Show resolved Hide resolved
Mathlib/Order/CompactlyGenerated/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Order/Filter/AtTopBot.lean Outdated Show resolved Hide resolved
Mathlib/RingTheory/Polynomial/ScaleRoots.lean Outdated Show resolved Hide resolved
time_startup.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

I've gone over the entire diff: apart from these few nits and the ones already mentioned, the changes are either mechanical, improvements or reverting temporary sorries.

Archive/Examples/IfNormalization/WithoutAesop.lean Outdated Show resolved Hide resolved
Archive/Wiedijk100Theorems/SolutionOfCubic.lean Outdated Show resolved Hide resolved
Archive/Wiedijk100Theorems/SolutionOfCubic.lean Outdated Show resolved Hide resolved
Mathlib/RingTheory/AlgebraTower.lean Show resolved Hide resolved
mathlib-bors bot pushed a commit that referenced this pull request Mar 1, 2024
A random collection of changes, backporting from the upcoming Lean bump.
- squeeze one simp
- backport one more simp change (which probably was missed in the previous backports)
- rewrite the `field_simp` in SolutionOfCubic: with the upcoming Lean bump, this would become **very** slow

Similar to #10996 and #11001.
@jcommelin
Copy link
Member

Thanks 🎉

If CI passes, please remove the label awaiting-CI and merge this yourself, by adding a comment bors r+.

bors d+

@mathlib-bors
Copy link

mathlib-bors bot commented Mar 1, 2024

✌️ 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.

@semorrison semorrison added auto-merge-after-CI Please do not add manually. Requests for a bot to merge automatically once CI is done. and removed awaiting-CI labels Mar 1, 2024
riccardobrasca pushed a commit that referenced this pull request Mar 1, 2024
A random collection of changes, backporting from the upcoming Lean bump.
- squeeze one simp
- backport one more simp change (which probably was missed in the previous backports)
- rewrite the `field_simp` in SolutionOfCubic: with the upcoming Lean bump, this would become **very** slow

Similar to #10996 and #11001.
@semorrison
Copy link
Contributor Author

bors p=10

@adomani
Copy link
Collaborator

adomani commented Mar 2, 2024

I just cherry-picked 51da0513b468f47b974b0aa874ca730371f70416, since it seems to fix compute_degree and not cause further breakage.

@leanprover-community-mathlib4-bot
Copy link
Collaborator

As this PR is labelled auto-merge-after-CI, we are now sending it to bors:

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Mar 2, 2024
`field_simp` is a very ambitious tactic, that tries to prove that it can clear denominators. Previously it was taking advantage of essentially a bug in the simp discharging framework, so that parts of its discharging strategy were not limited by `maxDischargeDepth` (so it could "keep trying"), but everyone else was limited (so `simp` didn't waste time exploring large trees of discharges). Now the `maxDischargeDepth` is enforced for everyone, so to keep `field_simp` working we have to increase its `maxDischargeDepth`. That makes it extremely slow. I've kept the `maxDischargeDepth` to a minimum, but still there are some `set_option maxHeartbeats` (all with notes attached).

After we've merged this, someone is going to have to rewrite `field_simp` to make it performant.



Co-authored-by: thorimur <68410468+thorimur@users.noreply.github.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Michael Rothgang <rothgami@math.hu-berlin.de>
Co-authored-by: adomani <adomani@gmail.com>
@mathlib-bors
Copy link

mathlib-bors bot commented Mar 2, 2024

Pull request successfully merged into bump/v4.7.0.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: adaptations for nightly-2024-02-29 [Merged by Bors] - chore: adaptations for nightly-2024-02-29 Mar 2, 2024
@mathlib-bors mathlib-bors bot closed this Mar 2, 2024
@mathlib-bors mathlib-bors bot deleted the bump/nightly-2024-02-29 branch March 2, 2024 19:42
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
A random collection of changes, backporting from the upcoming Lean bump.
- squeeze one simp
- backport one more simp change (which probably was missed in the previous backports)
- rewrite the `field_simp` in SolutionOfCubic: with the upcoming Lean bump, this would become **very** slow

Similar to #10996 and #11001.
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
A random collection of changes, backporting from the upcoming Lean bump.
- squeeze one simp
- backport one more simp change (which probably was missed in the previous backports)
- rewrite the `field_simp` in SolutionOfCubic: with the upcoming Lean bump, this would become **very** slow

Similar to #10996 and #11001.
utensil pushed a commit that referenced this pull request Mar 26, 2024
A random collection of changes, backporting from the upcoming Lean bump.
- squeeze one simp
- backport one more simp change (which probably was missed in the previous backports)
- rewrite the `field_simp` in SolutionOfCubic: with the upcoming Lean bump, this would become **very** slow

Similar to #10996 and #11001.
Louddy pushed a commit that referenced this pull request Apr 15, 2024
A random collection of changes, backporting from the upcoming Lean bump.
- squeeze one simp
- backport one more simp change (which probably was missed in the previous backports)
- rewrite the `field_simp` in SolutionOfCubic: with the upcoming Lean bump, this would become **very** slow

Similar to #10996 and #11001.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-after-CI Please do not add manually. Requests for a bot to merge automatically once CI is done. delegated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants