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] - perf(topology/sheaves/presheaf_of_functions): speedups #16873

Closed
wants to merge 3 commits into from

Conversation

alreadydone
Copy link
Collaborator

@alreadydone alreadydone commented Oct 8, 2022

Speed upCommRing_yoneda (which caused a bors failure) from >20s to <2.5s by filling in automatic fields. (merged in another PR)

Speed up presheaf_to_Types from 13.3s to 0.3s and presheaf_to_Type from 4s to 0.1s by filling in automatic fields.


Zulip

Open in Gitpod

@alreadydone alreadydone added easy < 20s of review time. See the lifecycle page for guidelines. awaiting-review The author would like community review of the PR labels Oct 8, 2022
@alreadydone
Copy link
Collaborator Author

maintainer merge

@github-actions
Copy link

github-actions bot commented Oct 8, 2022

🚀 Pull request has been placed on the maintainer queue by alreadydone.

@kbuzzard
Copy link
Member

kbuzzard commented Oct 8, 2022

Thanks! This declaration was causing unrelated timeouts in several other places (e.g. #13250 and #16721)

@semorrison
Copy link
Collaborator

bors merge

@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 Oct 8, 2022
@alreadydone alreadydone added awaiting-review The author would like community review of the PR and removed ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) easy < 20s of review time. See the lifecycle page for guidelines. labels Oct 8, 2022
@bors
Copy link

bors bot commented Oct 8, 2022

Canceled.

@alreadydone
Copy link
Collaborator Author

Since this has been merged into #15663 which is in queue, let me take the opportunity of this PR to speedup to other declarations.

@alreadydone alreadydone changed the title perf(topology/sheaves/presheaf_of_functions): fix timeout perf(topology/sheaves/presheaf_of_functions): speedups Oct 9, 2022
@alreadydone alreadydone added the easy < 20s of review time. See the lifecycle page for guidelines. label Oct 9, 2022
@kbuzzard
Copy link
Member

kbuzzard commented Oct 9, 2022

You could leave a comment saying that tidy can do these but you're doing them anyway, but it's probably better to get this merged ASAP

@alreadydone
Copy link
Collaborator Author

You could leave a comment saying that tidy can do these but you're doing them anyway, but it's probably better to get this merged ASAP

I think in general it's reasonable to prefer longer, less-automatic but much faster code over shorter but slower code, and does not need a special explanation. And it's clearly not rare to fill in the map_id'/comp' fields manually as can be seen from the search results.

By the way, I just found out you actually beat me in opening PR (#16872) and had exactly the same changes!

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Oct 10, 2022
bors bot pushed a commit that referenced this pull request Oct 10, 2022
Speed up`CommRing_yoneda` (which caused [a bors failure](https://github.com/leanprover-community/mathlib/actions/runs/3211416019/jobs/5249570763)) from >20s to <2.5s by filling in automatic fields. (merged in another PR)

Speed up `presheaf_to_Types` from 13.3s to 0.3s and `presheaf_to_Type` from 4s to 0.1s by filling in automatic fields.
@bors
Copy link

bors bot commented Oct 10, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title perf(topology/sheaves/presheaf_of_functions): speedups [Merged by Bors] - perf(topology/sheaves/presheaf_of_functions): speedups Oct 10, 2022
@bors bors bot closed this Oct 10, 2022
@bors bors bot deleted the presheaf_of_functions_timeout branch October 10, 2022 08:47
@eric-wieser eric-wieser added the hacktoberfest-accepted Without this label hacktoberfest is scared off by bors label Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy < 20s of review time. See the lifecycle page for guidelines. hacktoberfest-accepted Without this label hacktoberfest is scared off by bors 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

6 participants