Skip to content

Cut down runtime of rolling-ball gallery example.#7705

Merged
mkcor merged 12 commits intoscikit-image:mainfrom
mkcor:improve-rolling-example
Mar 7, 2025
Merged

Cut down runtime of rolling-ball gallery example.#7705
mkcor merged 12 commits intoscikit-image:mainfrom
mkcor:improve-rolling-example

Conversation

@mkcor
Copy link
Copy Markdown
Member

@mkcor mkcor commented Feb 18, 2025

Description

This PR follows up on #7682.

Closes #7680.

Checklist

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

...

@mkcor mkcor added 📄 type: Documentation Updates, fixes and additions to documentation 🔧 type: Maintenance Refactoring and maintenance of internals and removed 🔧 type: Maintenance Refactoring and maintenance of internals labels Feb 18, 2025
@mkcor mkcor marked this pull request as ready for review February 18, 2025 22:33
background2 = restoration.rolling_ball(x, radius=10)
background = ski.restoration.rolling_ball(x, radius=80)
background2 = ski.restoration.rolling_ball(x, radius=10)
plt.figure()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should update to Matplotlib's OO style here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you feel like it, but orthogonal to the issue I think. :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, totally orthogonal to this PR! I meant, it should be addressed in a different PR. In #7531, @GParolini took care of this for a few examples in doc/examples/segmentation but not all of them.

@mkcor mkcor requested a review from stefanv February 18, 2025 22:39
Copy link
Copy Markdown
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

This looks good so far @mkcor, although I missed where the optimization was happening; would have to go search for it more carefully.

mkcor and others added 2 commits February 27, 2025 16:12
Co-authored-by: Stefan van der Walt <stefanv@berkeley.edu>
@mkcor
Copy link
Copy Markdown
Member Author

mkcor commented Feb 27, 2025

I missed where the optimization was happening

It's happening here 071768d; in the PR description I linked to the exploration I did with %time to figure out which lines of code were taking a lot of time to execute.

Copy link
Copy Markdown
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

One tiny suggestion, otherwise LGTM. Go ahead and merge when you're ready. Thanks @mkcor!

Co-authored-by: Stefan van der Walt <stefan@mentat.za.net>
Copy link
Copy Markdown
Member

@lagru lagru left a comment

Choose a reason for hiding this comment

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

This shaves the runtime down from 53.82 s to 19.50 s (CI measurements). Very nice improvement 😊 and a bit annoying that it is still takes this long. 🐌

@mkcor mkcor merged commit b001f4b into scikit-image:main Mar 7, 2025
21 of 23 checks passed
@stefanv stefanv added this to the 0.26 milestone Mar 7, 2025
@mkcor mkcor deleted the improve-rolling-example branch March 7, 2025 13:16
@lagru lagru added 🔧 type: Maintenance Refactoring and maintenance of internals and removed 📄 type: Documentation Updates, fixes and additions to documentation labels Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔧 type: Maintenance Refactoring and maintenance of internals

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce runtime of rolling ball gallery example

3 participants