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

Save parameters to in-memory storage between update-steps #4026

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

ManInFez
Copy link
Contributor

@ManInFez ManInFez commented Oct 10, 2022

With localization in ensemble smoother, intermediate update results was stored to disk. This entangled the usage of storage with running analysis. This commit keeps all parameters in memory while running analysis, and then persisting to disk when all steps are finished.

Removing specialized functions for loading, saving and copying parameters

Removing irrelevant tests

Issue
Resolves #3377

Approach
Short description of the approach

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@ManInFez ManInFez force-pushed the store_param_test_storage branch 2 times, most recently from 936f15f to ead7a12 Compare October 10, 2022 10:19
@ManInFez ManInFez self-assigned this Oct 10, 2022
@ManInFez ManInFez force-pushed the store_param_test_storage branch 3 times, most recently from 6ad0ecd to e84553c Compare October 10, 2022 11:57
@dafeda dafeda self-requested a review October 10, 2022 12:34
@ManInFez ManInFez added the release-notes:improvement Automatically categorise as improvement in release notes label Oct 10, 2022
def _save_to_temporary_storage(
temporary_storage: Dict[str, "npt.NDArray[np.double]"],
parameters: List[update.Parameter],
A: Union["npt.NDArray[np.double]", None],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should do what annotate-python-linting says here.

@@ -9,16 +9,27 @@ def globalIndex(i, j, k, nx=10, ny=10):
return i + nx * (j - 1) + nx * ny * (k - 1)


def readParameters(filename):
def readSeed(filename):
Copy link
Contributor

Choose a reason for hiding this comment

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

Use pythonesque function names, i.e., read_seed?

Copy link
Contributor

@dafeda dafeda 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 👍

With localization in ensemble smoother, intermediate update results
was stored to disk. This entangled the usage of storage with running
analysis. This commit keeps all parameters in memory while running
analysis, and then persisting to disk when all steps are finished.

Removing specialized functions for loading, saving and copying parameters

Removing irrelevant tests
@ManInFez
Copy link
Contributor Author

test this please!

@ManInFez ManInFez merged commit fd55933 into equinor:main Oct 11, 2022
@ManInFez ManInFez deleted the store_param_test_storage branch October 11, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:improvement Automatically categorise as improvement in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove analysis update writing to storage while updating
2 participants