Skip to content

Commit

Permalink
Propagate SETENV to forward model validation
Browse files Browse the repository at this point in the history
The settings implied through SETENV can affect how forward
model behave, and may also change how validation should be
performed.

This is in particular true for the Eclipse forward model steps
which try to validate the requested version. If a different
configuration of Eclipse is set through

  SETENV ECL100_SITE_CONFIG somefile.yml

(or for ECL300_SITE_CONFIG) this information must be conveyed
to the validation code.
  • Loading branch information
berland committed Dec 9, 2024
1 parent baeb4f5 commit 8e4030e
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 32 deletions.
9 changes: 7 additions & 2 deletions src/ert/config/ert_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,15 @@ def _create_list_of_forward_model_steps_to_run(
cls,
installed_steps: Dict[str, ForwardModelStep],
substitutions: Substitutions,
config_dict,
config_dict: dict,
) -> List[ForwardModelStep]:
errors = []
fm_steps = []

env_vars = {}
for key, val in config_dict.get("SETENV", []):
env_vars[key] = val

for fm_step_description in config_dict.get(ConfigKeys.FORWARD_MODEL, []):
if len(fm_step_description) > 1:
unsubstituted_step_name, args = fm_step_description
Expand Down Expand Up @@ -836,7 +841,7 @@ def _create_list_of_forward_model_steps_to_run(
skip_pre_experiment_validation=True,
)
job_json = substituted_json["jobList"][0]
fm_step.validate_pre_experiment(job_json)
fm_step.validate_pre_experiment(job_json, env_vars)
except ForwardModelStepValidationError as err:
errors.append(
ConfigValidationError.with_context(
Expand Down
4 changes: 3 additions & 1 deletion src/ert/config/forward_model_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ def convert_to_substitutions(cls, v: Dict[str, str]) -> Substitutions:
return v
return Substitutions(v)

def validate_pre_experiment(self, fm_step_json: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, fm_step_json: ForwardModelStepJSON, env_vars: Dict[str, str]
) -> None:
"""
Raise errors pertaining to the environment not being
as the forward model step requires it to be. For example
Expand Down
28 changes: 18 additions & 10 deletions src/ert/plugins/hook_implementations/forward_model_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import subprocess
from pathlib import Path
from textwrap import dedent
from typing import List, Literal, Optional, Type
from typing import Dict, List, Literal, Optional, Type

import yaml

Expand All @@ -14,7 +14,6 @@
ForwardModelStepValidationError,
plugin,
)
from ert.plugins import ErtPluginManager


class CarefulCopyFile(ForwardModelStepPlugin):
Expand Down Expand Up @@ -220,13 +219,17 @@ def __init__(self) -> None:
default_mapping={"<NUM_CPU>": 1, "<OPTS>": ""},
)

def validate_pre_experiment(self, _: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, _: ForwardModelStepJSON, env_vars: Dict[str, str]
) -> None:
if "<VERSION>" not in self.private_args:
raise ForwardModelStepValidationError(
"Forward model step ECLIPSE100 must be given a VERSION argument"
)
version = self.private_args["<VERSION>"]
available_versions = _available_eclrun_versions(simulator="eclipse")
available_versions = _available_eclrun_versions(
simulator="eclipse", env_vars=env_vars
)

if available_versions and version not in available_versions:
raise ForwardModelStepValidationError(
Expand Down Expand Up @@ -278,13 +281,17 @@ def __init__(self) -> None:
default_mapping={"<NUM_CPU>": 1, "<OPTS>": "", "<VERSION>": "version"},
)

def validate_pre_experiment(self, _: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, _: ForwardModelStepJSON, env_vars: Dict[str, str]
) -> None:
if "<VERSION>" not in self.private_args:
raise ForwardModelStepValidationError(
"Forward model step ECLIPSE300 must be given a VERSION argument"
)
version = self.private_args["<VERSION>"]
available_versions = _available_eclrun_versions(simulator="e300")
available_versions = _available_eclrun_versions(
simulator="e300", env_vars=env_vars
)
if available_versions and version not in available_versions:
raise ForwardModelStepValidationError(
f"Unavailable ECLIPSE300 version {version} current supported "
Expand Down Expand Up @@ -628,14 +635,15 @@ def installable_forward_model_steps() -> List[Type[ForwardModelStepPlugin]]:
return [*_UpperCaseFMSteps, *_LowerCaseFMSteps]


def _available_eclrun_versions(simulator: Literal["eclipse", "e300"]) -> List[str]:
def _available_eclrun_versions(
simulator: Literal["eclipse", "e300"], env_vars: Dict[str, str]
) -> List[str]:
if shutil.which("eclrun") is None:
return []
pm = ErtPluginManager()
ecl_config_path = (
pm.get_ecl100_config_path()
env_vars.get("ECL100_SITE_CONFIG")
if simulator == "eclipse"
else pm.get_ecl300_config_path()
else env_vars.get("ECL300_SITE_CONFIG")
)

if not ecl_config_path:
Expand Down
2 changes: 2 additions & 0 deletions test-data/ert/poly_example/poly_eval.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env python3
import json
import time


def _load_coeffs(filename):
Expand All @@ -16,3 +17,4 @@ def _evaluate(coeffs, x):
output = [_evaluate(coeffs, x) for x in range(10)]
with open("poly.out", "w", encoding="utf-8") as f:
f.write("\n".join(map(str, output)))
time.sleep(10)
73 changes: 54 additions & 19 deletions tests/ert/unit_tests/config/test_forward_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import stat
from pathlib import Path
from textwrap import dedent
from typing import Dict
from unittest.mock import patch

import pytest
Expand Down Expand Up @@ -595,9 +596,12 @@ def test_that_eclipse_jobs_check_version(eclipse_v, mock_eclrun):

ecl100_config_content = f"eclrun_env:\n PATH: {os.getcwd()}\n"
ecl300_config_content = f"eclrun_env:\n PATH: {os.getcwd()}\n"
ert_config_contents = (
f"NUM_REALIZATIONS 1\nFORWARD_MODEL ECLIPSE{eclipse_v} (<VERSION>=1)\n"
)
ert_config_contents = f"""NUM_REALIZATIONS 1
SETENV ECL100_SITE_CONFIG {ecl100_config_file_name}
SETENV ECL300_SITE_CONFIG {ecl300_config_file_name}
FORWARD_MODEL ECLIPSE{eclipse_v} (<VERSION>=1)"""

# Write config file
config_file_name = "test.ert"
Expand All @@ -606,17 +610,11 @@ def test_that_eclipse_jobs_check_version(eclipse_v, mock_eclrun):
Path(ecl100_config_file_name).write_text(ecl100_config_content, encoding="utf-8")
# Write ecl300_config file
Path(ecl300_config_file_name).write_text(ecl300_config_content, encoding="utf-8")
with patch(
"ert.plugins.hook_implementations.forward_model_steps.ErtPluginManager"
) as mock:
instance = mock.return_value
instance.get_ecl100_config_path.return_value = ecl100_config_file_name
instance.get_ecl300_config_path.return_value = ecl300_config_file_name
with pytest.raises(
ConfigValidationError,
match=rf".*Unavailable ECLIPSE{eclipse_v} version 1 current supported versions \['4', '2', '8'\].*",
):
_ = ErtConfig.with_plugins().from_file(config_file_name)
with pytest.raises(
ConfigValidationError,
match=rf".*Unavailable ECLIPSE{eclipse_v} version 1 current supported versions \['4', '2', '8'\].*",
):
ErtConfig.with_plugins().from_file(config_file_name)


@pytest.mark.skipif(shutil.which("eclrun") is not None, reason="eclrun is available")
Expand Down Expand Up @@ -669,7 +667,9 @@ def __init__(self):
command=["something", "<arg1>", "-f", "<arg2>", "<arg3>"],
)

def validate_pre_experiment(self, fm_step_json: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, fm_step_json: ForwardModelStepJSON, env_vars: Dict[str, str]
) -> None:
if set(self.private_args.keys()) != {"<arg1>", "<arg2>", "<arg3>"}:
raise ForwardModelStepValidationError("Bad")

Expand Down Expand Up @@ -893,6 +893,33 @@ def validate_pre_realization_run(
)


def test_that_plugin_forward_model_validation_sees_setenv(tmp_path):
(tmp_path / "test.ert").write_text(
"""
NUM_REALIZATIONS 1
SETENV FOO bar
FORWARD_MODEL FM1()
"""
)

class ExceptionThatWeWant(ForwardModelStepValidationError):
pass

class FM1(ForwardModelStepPlugin):
def __init__(self):
super().__init__(name="FM1", command=["dummy.sh"])

def validate_pre_experiment(
self, _: ForwardModelStepJSON, env_vars: Dict[str, str]
) -> None:
raise ExceptionThatWeWant(f'Found FOO={env_vars["FOO"]}')

with pytest.raises(ConfigValidationError, match=".*Found FOO=bar.*"):
ErtConfig.with_plugins(forward_model_step_classes=[FM1]).from_file(
tmp_path / "test.ert"
)


def test_that_plugin_forward_model_raises_pre_experiment_validation_error_early(
tmp_path,
):
Expand All @@ -911,7 +938,9 @@ class FM1(ForwardModelStepPlugin):
def __init__(self):
super().__init__(name="FM1", command=["the_executable.sh"])

def validate_pre_experiment(self, fm_step_json: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, fm_step_json: ForwardModelStepJSON, _: Dict[str, str]
) -> None:
if self.name != "FM1":
raise ForwardModelStepValidationError("Expected name to be FM1")

Expand All @@ -924,7 +953,9 @@ def __init__(self):
command=["the_executable.sh"],
)

def validate_pre_experiment(self, fm_step_json: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, fm_step_json: ForwardModelStepJSON, _: Dict[str, str]
) -> None:
if self.name != "FM2":
raise ForwardModelStepValidationError("Expected name to be FM2")

Expand Down Expand Up @@ -1011,7 +1042,9 @@ class FMWithAssertionError(ForwardModelStepPlugin):
def __init__(self):
super().__init__(name="FMWithAssertionError", command=["the_executable.sh"])

def validate_pre_experiment(self, fm_step_json: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, fm_step_json: ForwardModelStepJSON, _: dict
) -> None:
raise AssertionError("I should be a warning")

class FMWithFMStepValidationError(ForwardModelStepPlugin):
Expand All @@ -1021,7 +1054,9 @@ def __init__(self):
command=["the_executable.sh"],
)

def validate_pre_experiment(self, fm_step_json: ForwardModelStepJSON) -> None:
def validate_pre_experiment(
self, fm_step_json: ForwardModelStepJSON, _: dict
) -> None:
raise ForwardModelStepValidationError("I should not be a warning")

with (
Expand Down

0 comments on commit 8e4030e

Please sign in to comment.