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

speeding up check_windspeed_drift (test mean 980us -> 482us) #43

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

gabrielecalvo
Copy link
Collaborator

@gabrielecalvo gabrielecalvo commented Jan 13, 2025

Speeding up execution by avoiding copy and keeping only needed data around.

The time has been cut to half from:

------------------------------------------------------------- benchmark: 1 tests ------------------------------------------------------------
Name (time in us)                         Min         Max      Mean    StdDev    Median       IQR  Outliers  OPS (Kops/s)  Rounds  Iterations
---------------------------------------------------------------------------------------------------------------------------------------------
test_calc_rolling_windspeed_diff     830.3000  2,590.3000  980.2053  230.7853  900.2000  133.6000     23;23        1.0202     206           1
---------------------------------------------------------------------------------------------------------------------------------------------

to

------------------------------------------------------------ benchmark: 1 tests ------------------------------------------------------------
Name (time in us)                         Min         Max      Mean    StdDev    Median      IQR  Outliers  OPS (Kops/s)  Rounds  Iterations
--------------------------------------------------------------------------------------------------------------------------------------------
test_calc_rolling_windspeed_diff     369.5000  1,267.7000  425.4475  104.1946  390.4500  30.1000     31;54        2.3505     362           1
--------------------------------------------------------------------------------------------------------------------------------------------

@gabrielecalvo gabrielecalvo requested a review from aclerc January 13, 2025 14:59
@aclerc
Copy link
Contributor

aclerc commented Jan 15, 2025

Thanks @gabrielecalvo! Looks great, I will review asap. Did you happen to benchmark test_pre_post_pp_analysis_with_reversal? That would be relevant to the changes you made to the pp_raw_df function.

@gabrielecalvo
Copy link
Collaborator Author

Thanks @gabrielecalvo! Looks great, I will review asap. Did you happen to benchmark test_pre_post_pp_analysis_with_reversal? That would be relevant to the changes you made to the pp_raw_df function.

yes, the last commit I did exactly that. Used the pytest-benchmark library and iterated over the pp_raw_df to make it faster.
The reason I chose that function is because when running cProfile on smart_ole that seemed one of the hotpaths.

@aclerc
Copy link
Contributor

aclerc commented Jan 15, 2025

Thanks @gabrielecalvo! Looks great, I will review asap. Did you happen to benchmark test_pre_post_pp_analysis_with_reversal? That would be relevant to the changes you made to the pp_raw_df function.

yes, the last commit I did exactly that. Used the pytest-benchmark library and iterated over the pp_raw_df to make it faster. The reason I chose that function is because when running cProfile on smart_ole that seemed one of the hotpaths.

thanks, sorry I see that info in your commit message now

Copy link
Contributor

@aclerc aclerc left a comment

Choose a reason for hiding this comment

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

Very nice improvements and thank you for adding the new test test_calc_rolling_windspeed_diff

@aclerc aclerc merged commit 7982f9b into main Jan 15, 2025
2 checks passed
@aclerc aclerc deleted the gabe/speedups branch January 15, 2025 11:08
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