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: speed up by changing def to abbrev in RingTheory/PowerSeries/Basic #14913

Closed
wants to merge 5 commits into from

Conversation

rwst
Copy link
Collaborator

@rwst rwst commented Jul 19, 2024

Fixes a 7-10s build delay in RingTheory/PowerSeries/Basic, due to convert idling, by changing def to abbrev. Which uncovered a diamond (unwanted RatFunc.instCoePolynomial).
https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/compiling.20behaviour.20within.20one.20file


Open in Gitpod

@rwst rwst added the WIP Work in progress label Jul 19, 2024
Copy link

github-actions bot commented Jul 19, 2024

PR summary 17bc81bfa6

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

- PowerSeries

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

@mattrobball
Copy link
Collaborator

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 170f508.
There were significant changes against commit 2d49bad:

  Benchmark                               Metric         Change
  =============================================================
+ ~Mathlib.RingTheory.PowerSeries.Basic   instructions   -55.8%

@riccardobrasca
Copy link
Member

Thanks!

bors merge

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 19, 2024

👎 Rejected by label

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Jul 19, 2024
@riccardobrasca
Copy link
Member

Oh sorry, is this really still WIP?

Copy link
Collaborator

@mattrobball mattrobball left a comment

Choose a reason for hiding this comment

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

Please update the PR description with details of the changes. Thanks!

@mattrobball
Copy link
Collaborator

One change + PR update

@mattrobball
Copy link
Collaborator

Ok two more

@riccardobrasca
Copy link
Member

bors d+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 19, 2024

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

@rwst
Copy link
Collaborator Author

rwst commented Jul 20, 2024

bors r+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 20, 2024

👎 Rejected by label

@rwst rwst removed the WIP Work in progress label Jul 20, 2024
@riccardobrasca
Copy link
Member

@rwst Can you please take into account the last two comments? Thanks!

@rwst
Copy link
Collaborator Author

rwst commented Jul 20, 2024

@rwst Can you please take into account the last two comments? Thanks!

That was my sleepy alter ego, sorry.

@rwst
Copy link
Collaborator Author

rwst commented Jul 20, 2024

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Jul 20, 2024
…ies/Basic` (#14913)

Fixes a 7-10s build delay in `RingTheory/PowerSeries/Basic`, due to `convert` idling, by changing `def` to `abbrev`. Which uncovered a diamond (unwanted `RatFunc.instCoePolynomial`).
https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/compiling.20behaviour.20within.20one.20file



Co-authored-by: Matthew Ballard <[email protected]>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 20, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: speed up by changing def to abbrev in RingTheory/PowerSeries/Basic [Merged by Bors] - chore: speed up by changing def to abbrev in RingTheory/PowerSeries/Basic Jul 20, 2024
@mathlib-bors mathlib-bors bot closed this Jul 20, 2024
@mathlib-bors mathlib-bors bot deleted the rwst/misc002 branch July 20, 2024 15:54
judithludwig pushed a commit that referenced this pull request Jul 20, 2024
…ies/Basic` (#14913)

Fixes a 7-10s build delay in `RingTheory/PowerSeries/Basic`, due to `convert` idling, by changing `def` to `abbrev`. Which uncovered a diamond (unwanted `RatFunc.instCoePolynomial`).
https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/compiling.20behaviour.20within.20one.20file



Co-authored-by: Matthew Ballard <[email protected]>
@adomani adomani mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants