-
Notifications
You must be signed in to change notification settings - Fork 139
Remove LocalEnsemble.load_all_gen_kw_data() #12015
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -540,7 +540,11 @@ def test_that_es_mda_on_poly_case_matches_snapshot(snapshot): | |
| experiment = storage.get_experiment_by_name("es-mda") | ||
| for iter_nr in range(4): | ||
| ensemble = experiment.get_ensemble_by_name(f"iter-{iter_nr}") | ||
| data.append(ensemble.load_all_gen_kw_data()) | ||
| ensemble_data = ensemble.load_scalars().to_pandas().set_index("realization") | ||
| ensemble_data.columns.name = None | ||
| ensemble_data.index.name = "Realization" | ||
| ensemble_data = ensemble_data.sort_index(axis=1) | ||
| data.append(ensemble_data) | ||
| result = pd.concat( | ||
| data, | ||
| keys=[f"iter-{iter_}" for iter_ in range(len(data))], | ||
|
|
@@ -576,7 +580,11 @@ def test_that_enif_on_poly_case_matches_snapshot(snapshot): | |
| experiment = storage.get_experiment_by_name("enif") | ||
| for iter_nr in range(2): | ||
| ensemble = experiment.get_ensemble_by_name(f"iter-{iter_nr}") | ||
| data.append(ensemble.load_all_gen_kw_data()) | ||
| ensemble_data = ensemble.load_scalars().to_pandas().set_index("realization") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to have pandas here? Ie. maybe just update snapshot? |
||
| ensemble_data.columns.name = None | ||
| ensemble_data.index.name = "Realization" | ||
| ensemble_data = ensemble_data.sort_index(axis=1) | ||
| data.append(ensemble_data) | ||
| result = pd.concat( | ||
| data, | ||
| keys=[f"iter-{i}" for i in range(len(data))], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1068,40 +1068,69 @@ def test_load_gen_kw_not_sorted(storage, tmpdir, snapshot): | |
| ) | ||
|
|
||
| sample_prior(ensemble, range(ensemble_size), random_seed=1234) | ||
|
|
||
| data = ensemble.load_all_gen_kw_data() | ||
| data = ensemble.load_scalars().to_pandas().set_index("realization") | ||
| data.columns.name = None | ||
| data.index.name = "Realization" | ||
| data = data.sort_index(axis=1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we update snapshot instead?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That will change behavior. I don't think we want to do that in this PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure that I understand. What do you mean by behaviour?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we have to update the snapshot, the behavior/format has been altered. I don't think that should be done in this refactor-pr |
||
| snapshot.assert_match(data.round(12).to_csv(), "gen_kw_unsorted") | ||
|
|
||
|
|
||
| def test_gen_kw_collector(snake_oil_default_storage, snapshot): | ||
| data = snake_oil_default_storage.load_all_gen_kw_data() | ||
| data = snake_oil_default_storage.load_scalars().to_pandas().set_index("realization") | ||
| data.columns.name = None | ||
| data.index.name = "Realization" | ||
| data = data.sort_index(axis=1) | ||
| snapshot.assert_match(data.round(6).to_csv(), "gen_kw_collector.csv") | ||
|
|
||
| with pytest.raises(KeyError): | ||
| # realization 60: | ||
| _ = data.loc[60] | ||
|
|
||
| data = snake_oil_default_storage.load_all_gen_kw_data( | ||
| "SNAKE_OIL_PARAM", | ||
| )[["SNAKE_OIL_PARAM:OP1_PERSISTENCE", "SNAKE_OIL_PARAM:OP1_OFFSET"]] | ||
| data = ( | ||
| snake_oil_default_storage.load_scalars( | ||
| "SNAKE_OIL_PARAM", | ||
| ) | ||
| .to_pandas() | ||
| .set_index("realization") | ||
| ) | ||
| data.columns.name = None | ||
| data.index.name = "Realization" | ||
| data = data.sort_index(axis=1) | ||
| data = data[["SNAKE_OIL_PARAM:OP1_PERSISTENCE", "SNAKE_OIL_PARAM:OP1_OFFSET"]] | ||
| snapshot.assert_match(data.round(6).to_csv(), "gen_kw_collector_2.csv") | ||
|
|
||
| with pytest.raises(KeyError): | ||
| _ = data["SNAKE_OIL_PARAM:OP1_DIVERGENCE_SCALE"] | ||
|
|
||
| realization_index = 3 | ||
| data = snake_oil_default_storage.load_all_gen_kw_data( | ||
| "SNAKE_OIL_PARAM", | ||
| realization_index=realization_index, | ||
| )["SNAKE_OIL_PARAM:OP1_PERSISTENCE"] | ||
| data = ( | ||
| snake_oil_default_storage.load_scalars( | ||
| "SNAKE_OIL_PARAM", | ||
| realizations=[realization_index], | ||
| ) | ||
| .to_pandas() | ||
| .set_index("realization") | ||
| ) | ||
| data.columns.name = None | ||
| data.index.name = "Realization" | ||
| data = data.sort_index(axis=1) | ||
| data = data["SNAKE_OIL_PARAM:OP1_PERSISTENCE"] | ||
| snapshot.assert_match(data.round(6).to_csv(), "gen_kw_collector_3.csv") | ||
|
|
||
| non_existing_realization_index = 150 | ||
| with pytest.raises((IndexError, KeyError)): | ||
| _ = snake_oil_default_storage.load_all_gen_kw_data( | ||
| "SNAKE_OIL_PARAM", | ||
| realization_index=non_existing_realization_index, | ||
| )["SNAKE_OIL_PARAM:OP1_PERSISTENCE"] | ||
| data = ( | ||
| snake_oil_default_storage.load_scalars( | ||
| "SNAKE_OIL_PARAM", | ||
| realizations=[non_existing_realization_index], | ||
| ) | ||
| .to_pandas() | ||
| .set_index("realization") | ||
| ) | ||
| data.columns.name = None | ||
| data.index.name = "Realization" | ||
| data = data.sort_index(axis=1) | ||
| data = data["SNAKE_OIL_PARAM:OP1_PERSISTENCE"] | ||
|
|
||
|
|
||
| def test_keyword_type_checks(snake_oil_default_storage): | ||
|
|
@@ -1130,12 +1159,12 @@ def test_data_fetching_missing_key(snake_oil_case): | |
| empty_case = experiment.create_ensemble(name="new_case", ensemble_size=25) | ||
|
|
||
| data = [ | ||
| empty_case.load_all_gen_kw_data("nokey", None), | ||
| empty_case.load_scalars("nokey", None), | ||
| ] | ||
|
|
||
| for dataframe in data: | ||
| assert isinstance(dataframe, DataFrame) | ||
| assert dataframe.empty | ||
| assert isinstance(dataframe, pl.DataFrame) | ||
| assert dataframe.is_empty() | ||
|
|
||
|
|
||
| @dataclass | ||
|
|
||
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 is more or less the same as what was in
load_all_gen_kw_data(), but it will be removed as we rewrite csv_export to use polars instead of pandas. We should do this gradually and in increments.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.
@xjules
I will create an issue for this rewrite pandas -> polars, but it won't be done in this PR.