Update field_statistics to use individual ertbox per zone by oddvarlia · Pull Request #858 · equinor/subscript

oddvarlia · 2026-04-10T11:14:54Z

Closes: #842

codecov-commenter · 2026-04-18T13:07:13Z

Codecov Report

❌ Patch coverage is 72.22222% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.27%. Comparing base (3317bd1) to head (a7fdb52).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
src/subscript/field_statistics/field_statistics.py	72.22%	70 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #858      +/-   ##
==========================================
- Coverage   84.51%   84.27%   -0.25%     
==========================================
  Files          49       49              
  Lines        7349     7472     +123     
==========================================
+ Hits         6211     6297      +86     
- Misses       1138     1175      +37

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Some model file changes from previous version was required Documentation is updated.

alifbe

Not quite sure if the files in ensemble/share/grid_statistics/ are all needed.

In general, we should avoid code repetitions.

alifbe · 2026-07-23T07:16:11Z

+
+
+


[nitpick] These extra empty lines are not needed.

alifbe · 2026-07-23T11:17:44Z

+        "Valysar": "ertbox_Valysar"
+        "Therys":  "ertbox_Therys"
+        "Volon":   "ertbox_Volon"


Propose to keep all to lower case.

Suggested change

"Valysar": "ertbox_Valysar"

"Therys": "ertbox_Therys"

"Volon": "ertbox_Volon"

"Valysar": "ertbox_valysar"

"Therys": "ertbox_therys"

"Volon": "ertbox_volon"

alifbe · 2026-07-23T11:26:26Z

        # Relative path relative to ERT <RUN_PATH> for localisation of
        # initial ensemble of field parameters
-        initial_relative_path: "rms/output/aps"
+        initial_field_relative_path: "rms/output/aps"


Is this necessary? This will break backward compatibility and personally I think previous naming is ok since it is under temporary_ertbox_fields.

alifbe · 2026-07-23T11:27:13Z

 DEFAULT_RELATIVE_RESULT_PATH = "share/grid_statistics"
 GLOBAL_VARIABLES_FILE = "../../fmuconfig/output/global_variables.yml"
-ERTBOX_GRID_PATH = "../../rms/output/aps/ERTBOX.EGRID"
+# ERTBOX_GRID_PATH = "../../rms/output/aps"


Please remove it if it's not relevant anymore

alifbe · 2026-07-23T11:32:13Z

+def get_ertbox_size_per_zone(
+    field_stat_dict: dict,
+    ert_config_path: str | Path,
+):


It will be nice to specify output type as well here.

alifbe · 2026-07-23T12:00:28Z

+    (
+        _use_geogrid_fields,
+        _use_temporary_fields,
+        _nreal,
+        _iter_list,
+        _use_population_stdev,
+        _relative_path_ertbox_grids,
+        _ertbox_per_zone,
+        _ertbox_default,
+        _zone_code_names,
+        geo_zone_names_used,
+        geo_zone_conformity,
+        geo_facies_per_zone,
+        geo_geogrid_name,
+        geo_param_name_dict,
+        geo_disc_param_name_dict,
+        _field_init_path,
+        _field_param_per_zone_dict,
+    ) = get_specifications(
+        input_dict,
+        use_facies_per_zone=use_facies_per_zone,
+        geo_facies_per_zone=facies_per_zone,
+    )


For return variables that are not used, you can just use _ and *_

Or use slicing.

Suggested change

(

_use_geogrid_fields,

_use_temporary_fields,

_nreal,

_iter_list,

_use_population_stdev,

_relative_path_ertbox_grids,

_ertbox_per_zone,

_ertbox_default,

_zone_code_names,

geo_zone_names_used,

geo_zone_conformity,

geo_facies_per_zone,

geo_geogrid_name,

geo_param_name_dict,

geo_disc_param_name_dict,

_field_init_path,

_field_param_per_zone_dict,

) = get_specifications(

input_dict,

use_facies_per_zone=use_facies_per_zone,

geo_facies_per_zone=facies_per_zone,

)

(

*_,

geo_zone_names_used,

geo_zone_conformity,

geo_facies_per_zone,

geo_geogrid_name,

geo_param_name_dict,

geo_disc_param_name_dict,

_,

_,

) = get_specifications(

input_dict,

use_facies_per_zone=use_facies_per_zone,

geo_facies_per_zone=facies_per_zone,

)

alifbe · 2026-07-23T12:01:44Z

+    (
+        _use_geogrid_fields,
+        _use_temporary_fields,
+        _nreal,
+        _iter_list,
+        _use_population_stdev,
+        _relative_path_ertbox_grids,
+        _ertbox_per_zone,
+        _ertbox_default,
+        _zone_code_names,
+        _geo_zone_names_used,
+        _geo_zone_conformity,
+        _geo_facies_per_zone,
+        _geo_geogrid_name,
+        _geo_param_name_dict,
+        _geo_disc_param_name_dict,
+        field_init_path,
+        field_param_per_zone_dict,
+    ) = get_specifications(


Similar as above:

Suggested change

(

_use_geogrid_fields,

_use_temporary_fields,

_nreal,

_iter_list,

_use_population_stdev,

_relative_path_ertbox_grids,

_ertbox_per_zone,

_ertbox_default,

_zone_code_names,

_geo_zone_names_used,

_geo_zone_conformity,

_geo_facies_per_zone,

_geo_geogrid_name,

_geo_param_name_dict,

_geo_disc_param_name_dict,

field_init_path,

field_param_per_zone_dict,

) = get_specifications(

*_ , field_init_path, field_param_per_zone_dict, = get_specifications(

alifbe · 2026-07-23T12:04:59Z

+    key = "ertbox_default"
+    ertbox_default = None
+    if key in input_dict:
+        ertbox_default = input_dict[key]
+
+    key = "ertbox_per_zone"
+    ertbox_per_zone = None
+    if key in input_dict:
+        ertbox_per_zone = input_dict[key]
+
+    if not ertbox_default and not ertbox_per_zone:
+        raise KeyError(
+            "Ertbox grid must be specified either as a common "
+            "grid for all zones or individual one per zone"
+        )
+
+    key = "zone_code_names"
+    if key in input_dict:
+        zone_code_names = input_dict[key]
+    else:
+        raise KeyError(f"Missing keyword:  {key} specifying zone codes and zone names.")


Similar code with the one in get_ertbox_size_per zone

Please consider create a helper function for it.

alifbe · 2026-07-23T12:10:34Z

+        # Default is to use all zones
        key = "use_zones"
-        zone_names_used = copy.copy(list(zone_code_names.values()))
+        geo_zone_names_used = copy.copy(list(zone_code_names.values()))


I think copy.copy is redundant here. list() will automatically create a new list container and since the values are string (immutable)

alifbe · 2026-07-23T12:12:33Z

+# def copy_to_real0_dirs(
+#     field_stat: dict, result_path: Path | str, ens_path: Path | str
+# ) -> None:
+#     iteration_list = field_stat["iterations"]
+#     for iter in iteration_list:
+#         source_files = result_path / Path(f"ertbox--*_{iter}.roff")
+#         target_dir = ens_path / Path(f"realization-0/iter-{iter}/share/results/grids")
+#         print(f"Source_files:  {source_files}")
+#         print(f"Target dir: {target_dir}")
+#         for f in glob.glob(source_files.as_posix()):
+#             shutil.copy(f, target_dir.as_posix())
+#         source_file = result_path / Path("ertbox.roff")
+#         shutil.copy(source_file.as_posix(), target_dir.as_posix())


Please remove unused code blocks

oddvarlia marked this pull request as draft April 10, 2026 11:15

oddvarlia force-pushed the update_field_statistics_with_zone_dependent_ertbox_grid branch from 8777229 to 0512285 Compare April 18, 2026 12:56

oddvarlia force-pushed the update_field_statistics_with_zone_dependent_ertbox_grid branch 6 times, most recently from c7630cf to 335b994 Compare April 21, 2026 10:47

oddvarlia requested a review from alifbe April 21, 2026 15:00

oddvarlia marked this pull request as ready for review April 21, 2026 15:00

oddvarlia marked this pull request as draft April 30, 2026 13:15

oddvarlia force-pushed the update_field_statistics_with_zone_dependent_ertbox_grid branch 7 times, most recently from 4d70873 to c2443be Compare May 6, 2026 21:40

Update field_statistics to use individual ertbox per zone

a7fdb52

Some model file changes from previous version was required Documentation is updated.

oddvarlia force-pushed the update_field_statistics_with_zone_dependent_ertbox_grid branch from c2443be to a7fdb52 Compare May 6, 2026 21:43

oddvarlia marked this pull request as ready for review May 6, 2026 21:54

alifbe requested changes Jul 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update field_statistics to use individual ertbox per zone#858

Update field_statistics to use individual ertbox per zone#858
oddvarlia wants to merge 1 commit into
equinor:mainfrom
oddvarlia:update_field_statistics_with_zone_dependent_ertbox_grid

oddvarlia commented Apr 10, 2026

Uh oh!

codecov-commenter commented Apr 18, 2026 •

edited

Loading

Uh oh!

alifbe left a comment

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

alifbe Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

oddvarlia commented Apr 10, 2026

Uh oh!

codecov-commenter commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alifbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Apr 18, 2026 •

edited

Loading