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

rolling_mean_pad Performance Improvements #249

Merged
merged 5 commits into from
Jul 12, 2024

Conversation

TimothyWillard
Copy link
Contributor

Replaced custom rolling mean implementation using np.pad and np.mean with implementation based on scipy.ndimage.convolve1d. This implementation also maintains the window shrinking behavior of the current implementation by manually computing the last row.

Addresses GH-248, but there aren't any unit tests for rolling_mean_pad right now AFAICT so I would feel much more confident merging this in after GH-247 which adds unit tests for this function (and will probably generate some merge conflicts).

Replaced custom rolling mean implementation using `np.pad` and `np.mean`
with implementation based on `scipy.ndimage.convolve1d`. This
implementation also maintains the window shrinking behavior of the
current implementation by manually computing the last row.
@jcblemai
Copy link
Collaborator

Approved, thank you :)

@jcblemai jcblemai requested a review from emprzy July 11, 2024 11:55
@TimothyWillard
Copy link
Contributor Author

I've run these changes through the test suite written in GH-247 and now feel confident in merging this into main.

(venv) twillard@epid-iss-mbp ~/D/G/H/f/f/gempyor_pkg (performance/GH-248-rolling_mean_pad-speed)> pytest tests/utils/test_rolling_mean_pad.py
=============================================================================================== test session starts ================================================================================================
platform darwin -- Python 3.11.9, pytest-8.2.2, pluggy-1.5.0
rootdir: /Users/twillard/Desktop/GitHub/HopkinsIDD/flepiMoP/flepimop/gempyor_pkg
configfile: pyproject.toml
collected 50 items

tests/utils/test_rolling_mean_pad.py ..................................................                                                                                                                      [100%]

================================================================================================ 50 passed in 0.78s ================================================================================================

@jcblemai jcblemai merged commit 4e8522e into main Jul 12, 2024
1 check passed
@jcblemai jcblemai deleted the performance/GH-248-rolling_mean_pad-speed branch July 12, 2024 09:19
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.

None yet

4 participants