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

Place ensemble member number determination for blending inside forecast loop to prevent out of memory issues #273

Merged
merged 3 commits into from
Mar 18, 2022

Conversation

RubenImhoff
Copy link
Contributor

In the recently posted release, the blending code determines which NWP models will be combined with which nowcast ensemble members. It does this at once and creates a variable that contains [n_ens_members, n_timesteps, n_cascade_levels, y, x]. For a large number of time steps and ensemble members, this variable can become too big too handle.

To overcome this issue, this PR tries to implement this procedure within the forecast loop (so per time step instead of all at once), which highly reduces the memory requirements.

@codecov
Copy link

codecov bot commented Mar 14, 2022

Codecov Report

Merging #273 (a6fb7da) into master (2dbd3de) will increase coverage by 0.09%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   82.23%   82.32%   +0.09%     
==========================================
  Files         158      158              
  Lines       12117    12130      +13     
==========================================
+ Hits         9964     9986      +22     
+ Misses       2153     2144       -9     
Flag Coverage Δ
unit_tests 82.32% <89.47%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pysteps/tests/test_blending_steps.py 99.12% <ø> (ø)
pysteps/blending/steps.py 83.73% <89.47%> (-0.17%) ⬇️
pysteps/tests/test_exporters.py 100.00% <0.00%> (ø)
pysteps/io/exporters.py 55.12% <0.00%> (+2.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dbd3de...a6fb7da. Read the comment docs.

@RubenImhoff RubenImhoff added the enhancement New feature or request label Mar 14, 2022
Copy link
Member

@dnerini dnerini left a comment

Choose a reason for hiding this comment

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

@RubenImhoff I was able only to quickly scan through your changes, but unfortunately I could not look more in depth into it... Are there critical parts where you would need a more detailed feedback? Otherwise this looks to me, happy to see that this piece of code keeps improving!

ps: one small detail, can you maybe provide a more specific title to this PR? Thanks!

@RubenImhoff
Copy link
Contributor Author

Are there critical parts where you would need a more detailed feedback?

Not really, all my own tests seem fine so far. The only small critical point is that it may slow down the forecast loop a bit (but probably not significantly).

one small detail, can you maybe provide a more specific title to this PR?

Will do so!

@RubenImhoff RubenImhoff changed the title Changes to the steps blending implementation Place ensemble member number determination for blending inside forecast loop to prevent out of memory issues Mar 16, 2022
@dnerini
Copy link
Member

dnerini commented Mar 16, 2022

one last thought: since you mention the memory usage, would you care to look into a memory profiling for a single run? using memory_profiler would as easy as running

mprof run python <your example script>.py
mprof plot

with this you could easily compare the impact of your new version.

@RubenImhoff
Copy link
Contributor Author

New code:
image

Old code:
Out-of-memory error, requested >100 GB according to the command line (I doubt it was that much, but defenitely too much for my laptop)

@RubenImhoff RubenImhoff merged commit 8936236 into pySTEPS:master Mar 18, 2022
@RubenImhoff RubenImhoff deleted the steps_blending_changes branch July 4, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants