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

Implement streaming algorithm #6316

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Implement streaming algorithm #6316

merged 1 commit into from
Oct 16, 2023

Conversation

dafeda
Copy link
Contributor

@dafeda dafeda commented Oct 13, 2023

Instead of loading all parameter groups into memory before updating, the streaming algorithm loads one parameter group into memory, updates it, and writes it back to disk.
This is particularly useful when multiple large fields are updated.

Issue
Resolves #5905

Pre review checklist

  • Read through the code changes carefully after finishing work
  • Make sure tests pass locally (after every commit!)
  • Prepare changes in small commits for more convenient review (optional)
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Updated documentation
  • Ensured that unit tests are added for all new behavior (See
    Ground Rules),
    and changes to existing code have good test coverage.

Pre merge checklist

  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.

@dafeda dafeda self-assigned this Oct 13, 2023
@dafeda dafeda added improvement Something nice to have, that will make life easier for developers or users or both. release-notes:improvement Automatically categorise as improvement in release notes labels Oct 13, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2023

Codecov Report

Merging #6316 (613d1fe) into main (945a5c6) will decrease coverage by 0.04%.
Report is 3 commits behind head on main.
The diff coverage is 90.74%.

@@            Coverage Diff             @@
##             main    #6316      +/-   ##
==========================================
- Coverage   82.68%   82.65%   -0.04%     
==========================================
  Files         347      347              
  Lines       21330    21338       +8     
  Branches      862      862              
==========================================
  Hits        17637    17637              
- Misses       3394     3402       +8     
  Partials      299      299              
Files Coverage Δ
src/ert/storage/local_ensemble.py 99.41% <100.00%> (+0.01%) ⬆️
src/ert/analysis/_es_update.py 88.99% <90.19%> (+0.44%) ⬆️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dafeda dafeda force-pushed the streaming_algo branch 2 times, most recently from 8ffd6d9 to d0bd32c Compare October 16, 2023 06:58
Copy link
Contributor

@frode-aarstad frode-aarstad left a comment

Choose a reason for hiding this comment

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

LGTM, does not hurt if @oyvindeide takes a look too

Instead of loading all parameter groups into memory before updating,
the streaming algorithm loads one parameter group into memory,
updates it, and writes it back to disk.
This is particularly useful when multiple large fields are updated.
@dafeda
Copy link
Contributor Author

dafeda commented Oct 16, 2023

Based on input from @oyvindeide , I have removed the usage of netCDF4 to only update a subset of parameters.
This is premature optimization we believe.

@dafeda dafeda merged commit f9f34a1 into equinor:main Oct 16, 2023
44 checks passed
@dafeda dafeda deleted the streaming_algo branch October 16, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Something nice to have, that will make life easier for developers or users or both. release-notes:improvement Automatically categorise as improvement in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

High memory usage when updating fields and surfaces
3 participants