Skip to content

Commit

Permalink
Remove excess minimum realizations validation in baserunmodel
Browse files Browse the repository at this point in the history
This commit removes the additional validation `validate_active_realizations_count` which checked that the number of  realizations to run was greater than the minimum required realizations count.
This validation is already done in `model_factory::_setup_ensemble_experiment`
  • Loading branch information
jonathan-eq committed Nov 26, 2024
1 parent 26ae63e commit dc5ab03
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 83 deletions.
12 changes: 0 additions & 12 deletions src/ert/run_models/base_run_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ def __init__(
self.minimum_required_realizations = minimum_required_realizations
self.active_realizations = copy.copy(active_realizations)
self.start_iteration = start_iteration
self.validate_active_realizations_count()

def log_at_startup(self) -> None:
keys_to_drop = [
Expand Down Expand Up @@ -658,17 +657,6 @@ def rm_run_path(self) -> None:
if Path(run_path).exists():
shutil.rmtree(run_path)

def validate_active_realizations_count(self) -> None:
active_realizations_count = self.get_number_of_active_realizations()
min_realization_count = self.minimum_required_realizations

if active_realizations_count < min_realization_count:
raise ValueError(
f"Number of active realizations ({active_realizations_count}) is less "
f"than the specified MIN_REALIZATIONS"
f"({min_realization_count})"
)

def validate_successful_realizations_count(self) -> None:
successful_reallizations_count = self.get_number_of_successful_realizations()
min_realization_count = self.minimum_required_realizations
Expand Down
57 changes: 21 additions & 36 deletions src/ert/run_models/model_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,29 +54,28 @@ def create_model(

if args.mode == TEST_RUN_MODE:
return _setup_single_test_run(config, storage, args, status_queue)
elif args.mode == ENSEMBLE_EXPERIMENT_MODE:
validate_minimum_realizations(config, args)
if args.mode == ENSEMBLE_EXPERIMENT_MODE:
return _setup_ensemble_experiment(config, storage, args, status_queue)
elif args.mode == EVALUATE_ENSEMBLE_MODE:
if args.mode == EVALUATE_ENSEMBLE_MODE:
return _setup_evaluate_ensemble(config, storage, args, status_queue)
elif args.mode == ENSEMBLE_SMOOTHER_MODE:
if args.mode == ENSEMBLE_SMOOTHER_MODE:
return _setup_ensemble_smoother(
config, storage, args, update_settings, status_queue
)
elif args.mode == ES_MDA_MODE:
if args.mode == ES_MDA_MODE:
return _setup_multiple_data_assimilation(
config, storage, args, update_settings, status_queue
)
elif args.mode == ITERATIVE_ENSEMBLE_SMOOTHER_MODE:
if args.mode == ITERATIVE_ENSEMBLE_SMOOTHER_MODE:
return _setup_iterative_ensemble_smoother(
config, storage, args, update_settings, status_queue
)
elif args.mode == MANUAL_UPDATE_MODE:
if args.mode == MANUAL_UPDATE_MODE:
return _setup_manual_update(
config, storage, args, update_settings, status_queue
)

else:
raise NotImplementedError(f"Run type not supported {args.mode}")
raise NotImplementedError(f"Run type not supported {args.mode}")


def _setup_single_test_run(
Expand All @@ -99,22 +98,26 @@ def _setup_single_test_run(
)


def _setup_ensemble_experiment(
config: ErtConfig,
storage: Storage,
args: Namespace,
status_queue: SimpleQueue[StatusEvents],
) -> EnsembleExperiment:
def validate_minimum_realizations(config: ErtConfig, args: Namespace) -> None:
min_realizations_count = config.analysis_config.minimum_required_realizations
active_realizations = _realizations(args, config.model_config.num_realizations)
active_realizations_count = int(np.sum(active_realizations))
if active_realizations_count < min_realizations_count:
config.analysis_config.minimum_required_realizations = active_realizations_count
ConfigWarning.warn(
f"Due to active_realizations {active_realizations_count} is lower than "
f"MIN_REALIZATIONS {min_realizations_count}, MIN_REALIZATIONS has been "
f"set to match active_realizations.",
"MIN_REALIZATIONS was set to the current number of active realizations "
f"({active_realizations_count}) as it is lower than the MIN_REALIZATIONS "
f"({min_realizations_count}) that was specified in the config file."
)


def _setup_ensemble_experiment(
config: ErtConfig,
storage: Storage,
args: Namespace,
status_queue: SimpleQueue[StatusEvents],
) -> EnsembleExperiment:
active_realizations = _realizations(args, config.model_config.num_realizations)
experiment_name = args.experiment_name
assert experiment_name is not None

Expand All @@ -137,16 +140,7 @@ def _setup_evaluate_ensemble(
args: Namespace,
status_queue: SimpleQueue[StatusEvents],
) -> EvaluateEnsemble:
min_realizations_count = config.analysis_config.minimum_required_realizations
active_realizations = _realizations(args, config.model_config.num_realizations)
active_realizations_count = int(np.sum(active_realizations))
if active_realizations_count < min_realizations_count:
config.analysis_config.minimum_required_realizations = active_realizations_count
ConfigWarning.warn(
"Adjusted MIN_REALIZATIONS to the current number of active realizations "
f"({active_realizations_count}) as it is lower than the MIN_REALIZATIONS "
f"({min_realizations_count}) that was specified in the config file."
)

return EvaluateEnsemble(
random_seed=config.random_seed,
Expand Down Expand Up @@ -178,16 +172,7 @@ def _setup_manual_update(
update_settings: UpdateSettings,
status_queue: SimpleQueue[StatusEvents],
) -> ManualUpdate:
min_realizations_count = config.analysis_config.minimum_required_realizations
active_realizations = _realizations(args, config.model_config.num_realizations)
active_realizations_count = int(np.sum(active_realizations))
if active_realizations_count < min_realizations_count:
config.analysis_config.minimum_required_realizations = active_realizations_count
ConfigWarning.warn(
"Adjusted MIN_REALIZATIONS to the current number of active realizations "
f"({active_realizations_count}) as it is lower than the MIN_REALIZATIONS "
f"({min_realizations_count}) that was specified in the config file."
)

return ManualUpdate(
random_seed=config.random_seed,
Expand Down
59 changes: 36 additions & 23 deletions tests/ert/ui_tests/cli/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,41 +180,52 @@ def test_unopenable_observation_config_fails_gracefully():
],
)
@pytest.mark.usefixtures("copy_poly_case")
def test_that_the_model_raises_exception_if_active_less_than_minimum_realizations(mode):
"""
Verify that the run model checks that active realizations 20 is less than 100
Omit testing of SingleTestRun because that executes with 1 active realization
regardless of configuration.
"""
def test_that_the_model_raises_exception_if_successful_realizations_less_than_minimum_realizations(
mode,
):
with (
open("poly.ert", "r", encoding="utf-8") as fin,
open("poly_high_min_reals.ert", "w", encoding="utf-8") as fout,
open("failing_realizations.ert", "w", encoding="utf-8") as fout,
):
for line in fin:
if "MIN_REALIZATIONS" in line:
fout.write("MIN_REALIZATIONS 100")
fout.write("MIN_REALIZATIONS 2\n")
elif "NUM_REALIZATIONS" in line:
fout.write("NUM_REALIZATIONS 2\n")
else:
fout.write(line)
fout.write(
dedent("""
INSTALL_JOB failing_fm FAILING_FM
FORWARD_MODEL failing_fm
""")
)
Path("FAILING_FM").write_text("EXECUTABLE failing_fm.py", encoding="utf-8")
Path("failing_fm.py").write_text(
"#!/usr/bin/env python3\nraise RuntimeError('fm failed')", encoding="utf-8"
)
os.chmod("failing_fm.py", 0o755)

with pytest.raises(
ErtCliError,
match="Number of active realizations",
match="Number of successful realizations",
):
run_cli(
mode,
"--disable-monitor",
"poly_high_min_reals.ert",
"--realizations",
"0-19",
"--target-case",
"testcase-%d",
)
run_cli(mode, "--disable-monitor", "failing_realizations.ert")


@pytest.mark.parametrize(
"mode",
[
pytest.param(ENSEMBLE_SMOOTHER_MODE),
pytest.param(ITERATIVE_ENSEMBLE_SMOOTHER_MODE),
pytest.param(ES_MDA_MODE),
],
)
@pytest.mark.usefixtures("copy_poly_case")
def test_that_the_model_warns_when_active_realizations_less_min_realizations():
def test_that_the_model_warns_when_active_realizations_less_min_realizations(mode):
"""
Verify that the run model checks that active realizations is equal or higher than
NUM_REALIZATIONS when running ensemble_experiment.
NUM_REALIZATIONS when running an experiment.
A warning is issued when NUM_REALIZATIONS is higher than active_realizations.
"""
with (
Expand All @@ -223,15 +234,17 @@ def test_that_the_model_warns_when_active_realizations_less_min_realizations():
):
for line in fin:
if "MIN_REALIZATIONS" in line:
fout.write("MIN_REALIZATIONS 100")
fout.write("MIN_REALIZATIONS 10\n")
elif "NUM_REALIZATIONS" in line:
fout.write("NUM_REALIZATIONS 10\n")
else:
fout.write(line)
with pytest.warns(
ConfigWarning,
match="Due to active_realizations 5 is lower than MIN_REALIZATIONS",
match=r"MIN_REALIZATIONS was set to the current number of active realizations \(5\)",
):
run_cli(
"ensemble_experiment",
mode,
"--disable-monitor",
"poly_lower_active_reals.ert",
"--realizations",
Expand Down
1 change: 0 additions & 1 deletion tests/ert/unit_tests/cli/test_model_hook_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
@pytest.fixture
def patch_base_run_model(monkeypatch):
monkeypatch.setattr(base_run_model, "create_run_path", MagicMock())
monkeypatch.setattr(BaseRunModel, "validate_active_realizations_count", MagicMock())
monkeypatch.setattr(
BaseRunModel, "validate_successful_realizations_count", MagicMock()
)
Expand Down
11 changes: 0 additions & 11 deletions tests/ert/unit_tests/run_models/test_model_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,6 @@ def test_multiple_data_assimilation_restart_paths(
experiment_name=None,
)

monkeypatch.setattr(
ert.run_models.base_run_model.BaseRunModel,
"validate_active_realizations_count",
MagicMock(),
)

monkeypatch.setattr(
ert.run_models.base_run_model.BaseRunModel,
"validate_successful_realizations_count",
Expand Down Expand Up @@ -269,11 +263,6 @@ def test_evaluate_ensemble_paths(
tmp_path, monkeypatch, ensemble_iteration, expected_path
):
monkeypatch.chdir(tmp_path)
monkeypatch.setattr(
ert.run_models.base_run_model.BaseRunModel,
"validate_active_realizations_count",
MagicMock(),
)

monkeypatch.setattr(
ert.run_models.base_run_model.BaseRunModel,
Expand Down

0 comments on commit dc5ab03

Please sign in to comment.