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(AlgebraicGeometry/GammaSpecAdjunction): speedup by adding universes #12469

Closed
wants to merge 3 commits into from

Conversation

kbuzzard
Copy link
Member

@kbuzzard kbuzzard commented Apr 27, 2024

Add some explicit universe annotations because they cause a speedup (currently an unexplained phenomenon, unfortunately).


Open in Gitpod

Ok so I don't really know what's going on here. The Zulip thread is here, with Ruben reporting that on recent nightlies the compilation time for this file had got worse, and it was already bad. When doing some experiments I added some explicit universe annotations (not because I thought they would help -- it was literally just to make a diff smaller) and one declaration just magically got much much faster. So I put set_option maxHeartbeats 20000 (note: not 200000) at the top of the file, a couple more declarations broke, and I added explicit universe annotations in all of them and they all got fixed. It would be good to get some kind of explanation of what's going on here because the universe unification problem looks to me like it should be trivial: there is only one universe at play in this file and the answer to all the universe unifications questions I saw was "set ?u.12345 = ?u.54321".

In comp_ring_hom_ext I guess there was a possibility that Lean was ultimately deciding that both universes should be equal but taking some time over it, and now we just tell it to assume this (note that the change doesn't change the type of the declaration).

@kbuzzard
Copy link
Member Author

!bench

@kbuzzard kbuzzard added awaiting-review The author would like community review of the PR t-algebraic-geometry Algebraic geometry labels Apr 27, 2024
@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 037b868.
There were significant changes against commit 8d0c6e6:

  Benchmark                                        Metric         Change
  ======================================================================
+ ~Mathlib.AlgebraicGeometry.GammaSpecAdjunction   instructions   -54.2%

@@ -460,7 +454,7 @@ theorem SpecΓIdentity_naturality {R S : CommRingCat} (f : R ⟶ S) :
(Scheme.Spec.map f.op).1.c.app (op ⊤) ≫ SpecΓIdentity.hom.app _ =
SpecΓIdentity.hom.app _ ≫ f := SpecΓIdentity.hom.naturality f

theorem SpecΓIdentity_hom_app_presheaf_obj {X : Scheme} (U : Opens X) :
theorem SpecΓIdentity_hom_app_presheaf_obj {X : Scheme.{u}} (U : Opens X) :
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably this one is worth stressing. I don't change the proof at all; I just make the universe explicit, and according to the profiler the total time taken on my machine goes from 0.7 secs to 0.23.

@semorrison
Copy link
Contributor

bors p=5

@semorrison
Copy link
Contributor

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 27, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 27, 2024
…es (#12469)

Add some explicit universe annotations because they cause a speedup (currently an unexplained phenomenon, unfortunately).



Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com>
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 27, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(AlgebraicGeometry/GammaSpecAdjunction): speedup by adding universes [Merged by Bors] - fix(AlgebraicGeometry/GammaSpecAdjunction): speedup by adding universes Apr 27, 2024
@mathlib-bors mathlib-bors bot closed this Apr 27, 2024
@mathlib-bors mathlib-bors bot deleted the kbuzzard-GammaSpecSpeedup branch April 27, 2024 23:58
mathlib-bors bot pushed a commit that referenced this pull request Apr 28, 2024
Removes a porting note resolved by #12469; clarifies a porting note touched by it.
mathlib-bors bot pushed a commit that referenced this pull request Apr 30, 2024
Following #12469 and #12494, we add explicit universe levels in `AlgebraicGeometry.Spec` to speed up the file now and forestall greater slowdowns with the next version of Lean.
apnelson1 pushed a commit that referenced this pull request May 12, 2024
…es (#12469)

Add some explicit universe annotations because they cause a speedup (currently an unexplained phenomenon, unfortunately).



Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com>
apnelson1 pushed a commit that referenced this pull request May 12, 2024
Removes a porting note resolved by #12469; clarifies a porting note touched by it.
apnelson1 pushed a commit that referenced this pull request May 12, 2024
Following #12469 and #12494, we add explicit universe levels in `AlgebraicGeometry.Spec` to speed up the file now and forestall greater slowdowns with the next version of Lean.
callesonne pushed a commit that referenced this pull request May 16, 2024
…es (#12469)

Add some explicit universe annotations because they cause a speedup (currently an unexplained phenomenon, unfortunately).



Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com>
callesonne pushed a commit that referenced this pull request May 16, 2024
Removes a porting note resolved by #12469; clarifies a porting note touched by it.
callesonne pushed a commit that referenced this pull request May 16, 2024
Following #12469 and #12494, we add explicit universe levels in `AlgebraicGeometry.Spec` to speed up the file now and forestall greater slowdowns with the next version of Lean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-algebraic-geometry Algebraic geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants