Skip to content

Commit 96d348a

Browse files
committed
Add allowed keywords to forward model steps
1 parent 1c4526b commit 96d348a

26 files changed

Lines changed: 210 additions & 13 deletions

File tree

docs/ert/getting_started/howto/plugin_system.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ To install forward model steps that you want to have available in ERT you can us
5858
super().__init__(
5959
name="MY_FORWARD_MODEL",
6060
command=["my_executable", "<parameter1>", "<parameter2>"],
61+
required_keywords=["<parameter1>"],
62+
allowed_keywords=["<parameter2>"],
6163
)
6264
6365
def validate_pre_realization_run(
@@ -94,6 +96,14 @@ forward model step is invalid (which ert then handles gracefully and presents ni
9496
to the user). If you want to show a warning in cases where the configuration cannot be
9597
validated pre-experiment, you can use the ``ForwardModelStepWarning.warn(...)`` method.
9698

99+
Use ``required_keywords`` to declare named arguments that the user must provide. Use
100+
``allowed_keywords`` to reject named arguments not supported by the forward model
101+
step. The allowed-keyword check is opt-in: omitting ``allowed_keywords`` preserves the
102+
default of accepting any named argument. Required keywords and keywords with a
103+
``default_mapping`` are implicitly allowed. Setting ``allowed_keywords`` to an empty
104+
list rejects all other named arguments. These declarations validate argument names
105+
only; use the validation methods for constraints on argument values.
106+
97107
.. code-block:: python
98108
99109
import ert

src/ert/config/ert_config.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -701,10 +701,17 @@ def create_list_of_forward_model_steps_to_run(
701701

702702
user_positional_args_by_step[id(fm_step)] = user_positional_args
703703

704-
try:
705-
fm_step.check_required_keywords()
706-
except ConfigValidationError as err:
707-
errors.append(err)
704+
keyword_errors: list[ConfigValidationError] = []
705+
for check_keywords in (
706+
fm_step.check_allowed_keywords,
707+
fm_step.check_required_keywords,
708+
):
709+
try:
710+
check_keywords()
711+
except ConfigValidationError as err:
712+
keyword_errors.append(err)
713+
if keyword_errors:
714+
errors.extend(keyword_errors)
708715
continue
709716
fm_steps.append(fm_step)
710717

src/ert/config/forward_model_step.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ class ForwardModelStepOptions(TypedDict, total=False):
9696
environment: NotRequired[dict[str, str]]
9797
default_mapping: NotRequired[dict[str, str]]
9898
required_keywords: NotRequired[list[str]]
99+
allowed_keywords: NotRequired[list[str] | None]
99100

100101

101102
def _get_source_package() -> str:
@@ -157,6 +158,9 @@ class ForwardModelStep(BaseModelWithContextSupport):
157158
max_arg: The maximum number of arguments
158159
arglist: The arglist with which the executable is invoked
159160
required_keywords: Keywords that are required to be supplied by the user
161+
allowed_keywords: Keywords that may be supplied by the user. If ``None``,
162+
any keyword is allowed; an empty list allows only required keywords and
163+
keywords with default values.
160164
arg_types: Types of user-provided arguments, this list must have indices
161165
corresponding to the number of args that are user specified
162166
and thus also subject to substitution.
@@ -182,6 +186,7 @@ class ForwardModelStep(BaseModelWithContextSupport):
182186
max_arg: int | None = None
183187
arglist: list[str] = Field(default_factory=list)
184188
required_keywords: list[str] = Field(default_factory=list)
189+
allowed_keywords: list[str] | None = None
185190
arg_types: list[SchemaItemType] = Field(default_factory=list)
186191
environment: dict[str, str] = Field(default_factory=dict)
187192
default_mapping: dict[str, str] = Field(default_factory=dict)
@@ -223,6 +228,27 @@ def check_required_keywords(self) -> None:
223228
self.name,
224229
)
225230

231+
def check_allowed_keywords(self) -> None:
232+
if self.allowed_keywords is None:
233+
return
234+
235+
accepted_keywords = (
236+
set(self.allowed_keywords)
237+
| set(self.required_keywords)
238+
| set(self.default_mapping)
239+
)
240+
unexpected_keywords = set(self.private_args).difference(accepted_keywords)
241+
if unexpected_keywords:
242+
plural = "s" if len(unexpected_keywords) > 1 else ""
243+
verb = "are" if plural else "is"
244+
allowed_keywords_message = ", ".join(sorted(accepted_keywords)) or "none"
245+
raise ConfigValidationError.with_context(
246+
f"Keyword{plural} {', '.join(sorted(unexpected_keywords))} "
247+
f"{verb} not allowed for forward model step {self.name}. "
248+
f"Allowed keywords: {allowed_keywords_message}",
249+
self.name,
250+
)
251+
226252
@model_validator(mode="after")
227253
def set_default_env(self) -> Self:
228254
self.environment.update(ForwardModelStep.default_env)
@@ -344,6 +370,7 @@ def __init__(
344370
environment = kwargs.get("environment", {}) or {}
345371
default_mapping = kwargs.get("default_mapping", {}) or {}
346372
required_keywords = kwargs.get("required_keywords", []) or []
373+
allowed_keywords = kwargs.get("allowed_keywords")
347374

348375
super().__init__(
349376
name=name,
@@ -359,6 +386,7 @@ def __init__(
359386
min_arg=0,
360387
max_arg=0,
361388
required_keywords=required_keywords,
389+
allowed_keywords=allowed_keywords,
362390
arg_types=[],
363391
environment=environment,
364392
default_mapping=default_mapping,

src/ert/plugins/hook_implementations/forward_model_steps.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
ForwardModelStepJSON,
1010
ForwardModelStepPlugin,
1111
ForwardModelStepValidationError,
12-
ForwardModelStepWarning,
1312
plugin,
1413
)
1514

@@ -216,6 +215,7 @@ def __init__(self) -> None:
216215
"<OPTS>",
217216
],
218217
default_mapping={"<NUM_CPU>": "1", "<OPTS>": ""},
218+
allowed_keywords=["<VERSION>", "<NUM_CPU>", "<OPTS>", "<ECLBASE>"],
219219
)
220220

221221
def validate_pre_experiment(self, fm_json: ForwardModelStepJSON) -> None:
@@ -281,6 +281,7 @@ def __init__(self) -> None:
281281
"<OPTS>",
282282
],
283283
default_mapping={"<NUM_CPU>": "1", "<OPTS>": "", "<VERSION>": "version"},
284+
allowed_keywords=["<VERSION>", "<NUM_CPU>", "<OPTS>", "<ECLBASE>"],
284285
)
285286

286287
def validate_pre_experiment(
@@ -344,10 +345,10 @@ def __init__(self) -> None:
344345
"<NUM_CPU>": "1",
345346
"<OPTS>": "",
346347
},
348+
allowed_keywords=["<VERSION>", "<NUM_CPU>", "<OPTS>", "<ECLBASE>"],
347349
)
348350

349351
def validate_pre_experiment(self, fm_json: ForwardModelStepJSON) -> None:
350-
allowed_args = {"<VERSION>", "<NUM_CPU>", "<OPTS>", "<ECLBASE>"}
351352
if "--np" in self.private_args.get("<OPTS>", ""):
352353
raise ForwardModelStepValidationError(
353354
"Do not supply --np as an option to FLOW, "
@@ -359,10 +360,6 @@ def validate_pre_experiment(self, fm_json: ForwardModelStepJSON) -> None:
359360
"set NUM_CPU in the Ert config instead."
360361
)
361362

362-
if unknowns := set(self.private_args) - allowed_args:
363-
raise ForwardModelStepWarning(
364-
f"Unknown option(s) supplied to Flow: {sorted(unknowns)}"
365-
)
366363
available_versions = _available_flow_versions(
367364
env_vars=fm_json["environment"] or {}
368365
)

tests/ert/unit_tests/config/test_forward_model.py

Lines changed: 123 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,16 @@ def test_that_flow_fm_gives_error_on_threads_in_opts(plugins_ert_config):
609609
)
610610

611611

612-
def test_that_flow_fm_gives_config_warning_on_unknown_options(plugins_ert_config):
613-
with pytest.warns(ConfigWarning, match=r".*Unknown option.*Flow: .*DUMMY.*"):
612+
@pytest.mark.parametrize("fm_step_name", ["ECLIPSE100", "ECLIPSE300", "FLOW"])
613+
def test_that_reservoir_simulator_fm_rejects_unknown_keyword(
614+
plugins_ert_config, fm_step_name
615+
):
616+
with pytest.raises(
617+
ConfigValidationError,
618+
match=rf"Keyword <DUMMY> is not allowed for forward model step {fm_step_name}",
619+
):
614620
plugins_ert_config.from_file_contents(
615-
"NUM_REALIZATIONS 1\nFORWARD_MODEL FLOW(<DUMMY>=moredummy)\n"
621+
f"NUM_REALIZATIONS 1\nFORWARD_MODEL {fm_step_name}(<DUMMY>=moredummy)\n"
616622
)
617623

618624

@@ -1126,6 +1132,120 @@ def __init__(self) -> None:
11261132
)
11271133

11281134

1135+
def _load_forward_model_with_keyword_contract(
1136+
invocation: str,
1137+
*,
1138+
allowed_keywords: list[str] | None,
1139+
required_keywords: list[str] | None = None,
1140+
default_mapping: dict[str, str] | None = None,
1141+
command: list[str] | None = None,
1142+
) -> ErtConfig:
1143+
class FMWithKeywordContract(ForwardModelStepPlugin):
1144+
def __init__(self) -> None:
1145+
super().__init__(
1146+
name="FMWithKeywordContract",
1147+
command=command or ["echo"],
1148+
required_keywords=required_keywords or [],
1149+
allowed_keywords=allowed_keywords,
1150+
default_mapping=default_mapping or {},
1151+
)
1152+
1153+
return ErtConfig.with_plugins(
1154+
ErtRuntimePlugins(
1155+
installed_forward_model_steps={
1156+
"FMWithKeywordContract": FMWithKeywordContract(),
1157+
}
1158+
)
1159+
).from_file_contents(
1160+
f"""
1161+
NUM_REALIZATIONS 1
1162+
FORWARD_MODEL FMWithKeywordContract{invocation}
1163+
"""
1164+
)
1165+
1166+
1167+
def test_that_plugin_fm_step_rejects_keyword_not_in_allowed_keywords():
1168+
with pytest.raises(
1169+
ConfigValidationError,
1170+
match=(
1171+
r"Keyword <C> is not allowed for forward model step "
1172+
r"FMWithKeywordContract\. Allowed keywords: <A>, <B>"
1173+
),
1174+
):
1175+
_load_forward_model_with_keyword_contract(
1176+
"(<A>=one,<C>=three)",
1177+
allowed_keywords=["<A>", "<B>"],
1178+
command=["echo", "<A>", "<B>"],
1179+
)
1180+
1181+
1182+
def test_that_plugin_fm_step_without_allowed_keywords_accepts_unknown_keywords():
1183+
_load_forward_model_with_keyword_contract(
1184+
"(<UNKNOWN>=value)",
1185+
allowed_keywords=None,
1186+
)
1187+
1188+
1189+
def test_that_plugin_fm_step_with_empty_allowed_keywords_rejects_named_argument():
1190+
with pytest.raises(
1191+
ConfigValidationError,
1192+
match=(
1193+
r"Keyword <A> is not allowed for forward model step "
1194+
r"FMWithKeywordContract\. Allowed keywords: none"
1195+
),
1196+
):
1197+
_load_forward_model_with_keyword_contract(
1198+
"(<A>=one)",
1199+
allowed_keywords=[],
1200+
)
1201+
1202+
1203+
def test_that_allowed_keywords_do_not_restrict_keyword_values():
1204+
ert_config = _load_forward_model_with_keyword_contract(
1205+
'(<OPTIONS>="--foo --bar=baz")',
1206+
allowed_keywords=["<OPTIONS>"],
1207+
command=["echo", "<OPTIONS>"],
1208+
)
1209+
1210+
assert ert_config.forward_model_steps[0].private_args == {
1211+
"<OPTIONS>": "--foo --bar=baz"
1212+
}
1213+
1214+
1215+
def test_that_required_keywords_are_implicitly_allowed():
1216+
_load_forward_model_with_keyword_contract(
1217+
"(<REQUIRED>=value)",
1218+
allowed_keywords=[],
1219+
required_keywords=["<REQUIRED>"],
1220+
command=["echo", "<REQUIRED>"],
1221+
)
1222+
1223+
1224+
def test_that_defaulted_keyword_is_implicitly_allowed():
1225+
ert_config = _load_forward_model_with_keyword_contract(
1226+
"(<DEFAULTED>=override)",
1227+
allowed_keywords=[],
1228+
default_mapping={"<DEFAULTED>": "default"},
1229+
command=["echo", "<DEFAULTED>"],
1230+
)
1231+
1232+
assert ert_config.forward_model_steps[0].private_args == {"<DEFAULTED>": "override"}
1233+
1234+
1235+
def test_that_required_keyword_typo_reports_unknown_and_missing_keywords():
1236+
with pytest.raises(ConfigValidationError) as exc_info:
1237+
_load_forward_model_with_keyword_contract(
1238+
"(<REQURIED>=value)",
1239+
allowed_keywords=[],
1240+
required_keywords=["<REQUIRED>"],
1241+
command=["echo", "<REQUIRED>"],
1242+
)
1243+
1244+
error = str(exc_info.value)
1245+
assert "Keyword <REQURIED> is not allowed" in error
1246+
assert "Required keyword <REQUIRED> not found" in error
1247+
1248+
11291249
@pytest.mark.usefixtures("use_tmpdir")
11301250
def test_that_one_required_keyword_in_forward_model_is_validated():
11311251
Path("step").write_text("EXECUTABLE echo\nREQUIRED MESSAGE", encoding="utf-8")

tests/ert/unit_tests/run_models/snapshots/test_experiment_serialization/test_that_dumped_enif_matches_snapshot/heat_equationconfig.ert/config.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"<ARG1>"
4949
],
5050
"required_keywords": [],
51+
"allowed_keywords": null,
5152
"arg_types": [],
5253
"environment": {
5354
"_ERT_ITERATION_NUMBER": "<ITER>",

tests/ert/unit_tests/run_models/snapshots/test_experiment_serialization/test_that_dumped_enif_matches_snapshot/poly_examplepoly.ert/poly.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"max_arg": null,
4242
"arglist": [],
4343
"required_keywords": [],
44+
"allowed_keywords": null,
4445
"arg_types": [],
4546
"environment": {
4647
"_ERT_ITERATION_NUMBER": "<ITER>",

tests/ert/unit_tests/run_models/snapshots/test_experiment_serialization/test_that_dumped_enif_matches_snapshot/snake_oilsnake_oil.ert/snake_oil.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"max_arg": null,
4242
"arglist": [],
4343
"required_keywords": [],
44+
"allowed_keywords": null,
4445
"arg_types": [],
4546
"environment": {
4647
"_ERT_ITERATION_NUMBER": "<ITER>",
@@ -65,6 +66,7 @@
6566
"max_arg": null,
6667
"arglist": [],
6768
"required_keywords": [],
69+
"allowed_keywords": null,
6870
"arg_types": [],
6971
"environment": {
7072
"_ERT_ITERATION_NUMBER": "<ITER>",
@@ -89,6 +91,7 @@
8991
"max_arg": null,
9092
"arglist": [],
9193
"required_keywords": [],
94+
"allowed_keywords": null,
9295
"arg_types": [],
9396
"environment": {
9497
"_ERT_ITERATION_NUMBER": "<ITER>",

tests/ert/unit_tests/run_models/snapshots/test_experiment_serialization/test_that_dumped_ensemble_experiment_matches_snapshot/heat_equationconfig.ert/config.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"<ARG1>"
4949
],
5050
"required_keywords": [],
51+
"allowed_keywords": null,
5152
"arg_types": [],
5253
"environment": {
5354
"_ERT_ITERATION_NUMBER": "<ITER>",

tests/ert/unit_tests/run_models/snapshots/test_experiment_serialization/test_that_dumped_ensemble_experiment_matches_snapshot/poly_examplepoly.ert/poly.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"max_arg": null,
4242
"arglist": [],
4343
"required_keywords": [],
44+
"allowed_keywords": null,
4445
"arg_types": [],
4546
"environment": {
4647
"_ERT_ITERATION_NUMBER": "<ITER>",

0 commit comments

Comments
 (0)