Skip to content

Parallelize rows in snip1d#777

Merged
saransh13 merged 1 commit intomasterfrom
parallelize-snip1d
Mar 4, 2025
Merged

Parallelize rows in snip1d#777
saransh13 merged 1 commit intomasterfrom
parallelize-snip1d

Conversation

@psavery
Copy link
Copy Markdown
Collaborator

@psavery psavery commented Feb 27, 2025

Snip1d was taking a very long time to compute for high resolution polar images. Since it is computed on a row-by-row basis, however, I found that parallelizing over the rows using multiprocessing provided a big performance improvement (3x faster for my example).

Some other things I tried that did not result in a speed improvement:

  • Using the latest convolve from astropy
  • Using the latest convolve_fft from astropy
  • Performing multithreading instead of multiprocessing
  • Parallelizing at a finer grain level in the snip1d loops

This also includes a test that verifies that the snip1d output does not change.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 39.28571% with 17 lines in your changes missing coverage. Please review.

Project coverage is 46.75%. Comparing base (2e193cf) to head (9a3c34c).
Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
hexrd/imageutil.py 44.00% 14 Missing ⚠️
hexrd/fitting/fitpeak.py 0.00% 2 Missing ⚠️
hexrd/fitting/spectrum.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #777      +/-   ##
==========================================
+ Coverage   46.67%   46.75%   +0.07%     
==========================================
  Files         142      142              
  Lines       22826    22839      +13     
==========================================
+ Hits        10655    10679      +24     
+ Misses      12171    12160      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@psavery psavery force-pushed the parallelize-snip1d branch 5 times, most recently from 305cdbd to cff6b3e Compare February 28, 2025 19:44
Snip1d was taking a very long time to compute for high resolution polar
images. Since it is computed on a row-by-row basis, however, I found that
parallelizing over the rows using multiprocessing provided a big performance
improvement (3x faster for my example).

Some other things I tried that did not result in a speed improvement:

* Using the latest `convolve` from astropy
* Using the latest `convolve_fft` from astropy
* Performing multithreading instead of multiprocessing
* Parallelizing at a finer grain level in the snip1d loops

This also includes a test that verifies that the snip1d output does
not change.

Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
Copy link
Copy Markdown
Member

@saransh13 saransh13 left a comment

Choose a reason for hiding this comment

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

We should add same parallelism to the FastSNIP1D kernel in a future PR.

@saransh13 saransh13 merged commit 2baefd9 into master Mar 4, 2025
7 checks passed
@psavery psavery deleted the parallelize-snip1d branch April 30, 2025 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants