-
Notifications
You must be signed in to change notification settings - Fork 18
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
performance optimize _generate_fourier_series_np
#516
performance optimize _generate_fourier_series_np
#516
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #516 +/- ##
==========================================
+ Coverage 49.73% 49.85% +0.12%
==========================================
Files 50 50
Lines 3557 3566 +9
==========================================
+ Hits 1769 1778 +9
Misses 1788 1788
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
# assume mse smaller eps is 'perfect' - only relevant for noiseless test data | ||
mse = mse if mse > np.finfo(float).eps else 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This suggestion may be relevant for https://github.com/MESMER-group/mesmer/pull/501/files#r1738976265
CHANGELOG.rst
Outdated
`#415 <https://github.com/MESMER-group/mesmer/pull/415>`_, | ||
`#424 <https://github.com/MESMER-group/mesmer/pull/424>`_, and | ||
`#433 <https://github.com/MESMER-group/mesmer/pull/433>`_) | ||
- Refactor variable names, small code improvements, fixes and clean docstring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Refactor variable names, small code improvements, fixes and clean docstring | |
- Refactor variable names, small code improvements, optimization, fixes and clean docstring |
Thanks for the review. Let's merge. |
_generate_fourier_series_np
#410CHANGELOG.rst
Performance optimization of
_generate_fourier_series_np
. Builds on #512 so the diff is actually smaller. This does two main things:cos(alpha)
is periodic - so we only need to compute it once for each month (instead ofn_years * 12
times). We then reshape theyearly_predictor
ton_years x 12
such that broadcasting ensures each element is multiplied with the coefficientswe do
This speeds up
_generate_fourier_series_np
about 9 times andfit_harmonic_model
about 4 times (on my machine). We do pay this with a small loss in precision (see changes in tests)What we do additionally:
@
to compute theproduct-sum
alpha
array for the cost of some readabilityWhat else could we do:
cos_sin
array similar yeo-johnson: performance optimization #496fft
to get better inital conditions (fourier series: more consistent with standard definition #512 (comment))