From 1120a226f034f63a6c0e6d67054d1129ce3e02f5 Mon Sep 17 00:00:00 2001 From: DanSava Date: Thu, 19 Dec 2024 09:54:52 +0200 Subject: [PATCH] Remove SIMULATION_JOB keyword --- .../reference/configuration/forward_model.rst | 4 - docs/ert/reference/configuration/keywords.rst | 10 -- src/ert/config/ert_config.py | 23 +--- src/ert/config/parsing/config_keywords.py | 1 - src/ert/config/parsing/config_schema.py | 9 -- src/ert/config/summary_config.py | 3 +- src/everest/simulator/everest_to_ert.py | 8 +- test-data/ert/heat_equation/HEAT_EQUATION | 1 + test-data/ert/heat_equation/config.ert | 2 +- .../config_forward_init_false.ert | 2 +- tests/ert/ui_tests/cli/test_local_driver.py | 2 +- .../config/config_dict_generator.py | 9 -- .../config/test_create_forward_model_json.py | 75 ------------- .../ert/unit_tests/config/test_ert_config.py | 12 +- tests/everest/test_detached.py | 2 +- tests/everest/test_egg_simulation.py | 103 ++++++++++-------- tests/everest/test_res_initialization.py | 2 +- 17 files changed, 75 insertions(+), 193 deletions(-) diff --git a/docs/ert/reference/configuration/forward_model.rst b/docs/ert/reference/configuration/forward_model.rst index bd5fbfd7331..8f10dc51087 100644 --- a/docs/ert/reference/configuration/forward_model.rst +++ b/docs/ert/reference/configuration/forward_model.rst @@ -23,10 +23,6 @@ To add a job to the forward model, use the :code:`FORWARD_MODEL` keyword. Each :code:`FORWARD_MODEL` keyword instructs ERT to run a specific executable. You can build a series of jobs by listing multiple :code:`FORWARD_MODEL` keywords. -An alternative to :code:`FORWARD_MODEL` is the :code:`SIMULATION_JOB` keyword, -which can also configure the forward model. -The difference lies in how these keywords pass command-line arguments to the final executable. - You can find all pre-configured jobs to define your forward models :ref:`here `. These jobs form the building blocks for your custom forward models in ERT. diff --git a/docs/ert/reference/configuration/keywords.rst b/docs/ert/reference/configuration/keywords.rst index e07a436c0ae..beb27b88687 100644 --- a/docs/ert/reference/configuration/keywords.rst +++ b/docs/ert/reference/configuration/keywords.rst @@ -67,7 +67,6 @@ Keyword name Required :ref:`RUNPATH_FILE ` NO .ert_runpath_list Name of file with path for all forward models that ERT has run. To be used by user defined scripts to find the realizations :ref:`RUN_TEMPLATE ` NO Install arbitrary files in the runpath directory :ref:`SETENV ` NO You can modify the UNIX environment with SETENV calls -:ref:`SIMULATION_JOB ` NO Lightweight alternative FORWARD_MODEL :ref:`STOP_LONG_RUNNING ` NO FALSE Stop long running realizations after minimum number of realizations (MIN_REALIZATIONS) have run :ref:`SUBMIT_SLEEP ` NO 0.0 Determines for how long the system will sleep between submitting jobs. :ref:`SUMMARY ` NO Add summary variables for internalization @@ -1759,15 +1758,6 @@ FORWARD_MODEL In available jobs in ERT you can see a list of the jobs which are available. -SIMULATION_JOB --------------- -.. _simulation_job: - -``SIMULATION_JOB`` is a lightweight version of ``FORWARD_MODEL`` that allows passing -raw command line arguments to executable. -It is heavily used in Everest as the Everest configuration transpiles all jobs -into ``SIMULATION_JOB``. - JOB_SCRIPT ---------- .. _job_script: diff --git a/src/ert/config/ert_config.py b/src/ert/config/ert_config.py index 3042795454d..7822d29bb04 100644 --- a/src/ert/config/ert_config.py +++ b/src/ert/config/ert_config.py @@ -760,8 +760,12 @@ def _create_list_of_forward_model_steps_to_run( ) continue fm_step.private_args = Substitutions() - for key, val in args: - fm_step.private_args[key] = val + for arg in args: + match arg: + case key, val: + fm_step.private_args[key] = val + case val: + fm_step.arglist.append(val) should_add_step = True @@ -779,21 +783,6 @@ def _create_list_of_forward_model_steps_to_run( if should_add_step: fm_steps.append(fm_step) - for fm_step_description in config_dict.get(ConfigKeys.SIMULATION_JOB, []): - try: - fm_step = copy.deepcopy(installed_steps[fm_step_description[0]]) - except KeyError: - errors.append( - ConfigValidationError.with_context( - f"Could not find forward model step {fm_step_description[0]!r} " - f"in list of installed forward model steps: {installed_steps}", - fm_step_description[0], - ) - ) - continue - fm_step.arglist = fm_step_description[1:] - fm_steps.append(fm_step) - for fm_step in fm_steps: if fm_step.name in cls.PREINSTALLED_FORWARD_MODEL_STEPS: try: diff --git a/src/ert/config/parsing/config_keywords.py b/src/ert/config/parsing/config_keywords.py index 3cf8e46839e..be17832d3f1 100644 --- a/src/ert/config/parsing/config_keywords.py +++ b/src/ert/config/parsing/config_keywords.py @@ -40,7 +40,6 @@ class ConfigKeys(StrEnum): RUN_TEMPLATE = "RUN_TEMPLATE" SCHEDULE_PREDICTION_FILE = "SCHEDULE_PREDICTION_FILE" SETENV = "SETENV" - SIMULATION_JOB = "SIMULATION_JOB" STD_CUTOFF = "STD_CUTOFF" SUMMARY = "SUMMARY" SURFACE = "SURFACE" diff --git a/src/ert/config/parsing/config_schema.py b/src/ert/config/parsing/config_schema.py index 959566bf233..142b7f6bb70 100644 --- a/src/ert/config/parsing/config_schema.py +++ b/src/ert/config/parsing/config_schema.py @@ -49,14 +49,6 @@ def forward_model_keyword() -> SchemaItem: ) -def simulation_job_keyword() -> SchemaItem: - return SchemaItem( - kw=ConfigKeys.SIMULATION_JOB, - argc_max=None, - multi_occurrence=True, - ) - - def data_kw_keyword() -> SchemaItem: return SchemaItem( kw=ConfigKeys.DATA_KW, @@ -349,7 +341,6 @@ def init_user_config_schema() -> ConfigSchemaDict: path_keyword(ConfigKeys.ENSPATH), single_arg_keyword(ConfigKeys.JOBNAME), forward_model_keyword(), - simulation_job_keyword(), data_kw_keyword(), define_keyword(), existing_path_keyword(ConfigKeys.OBS_CONFIG), diff --git a/src/ert/config/summary_config.py b/src/ert/config/summary_config.py index 142cd9c58b9..fa6e2843156 100644 --- a/src/ert/config/summary_config.py +++ b/src/ert/config/summary_config.py @@ -80,8 +80,7 @@ def from_config_dict(cls, config_dict: ConfigDict) -> SummaryConfig | None: ) time_map = set(refcase.dates) if refcase is not None else None fm_steps = config_dict.get(ConfigKeys.FORWARD_MODEL, []) - sim_steps = config_dict.get(ConfigKeys.SIMULATION_JOB, []) - names = [fm_step[0] for fm_step in fm_steps + sim_steps] + names = [fm_step[0] for fm_step in fm_steps] simulation_step_exists = any( any(sim in _name.lower() for sim in ["eclipse", "flow"]) for _name in names diff --git a/src/everest/simulator/everest_to_ert.py b/src/everest/simulator/everest_to_ert.py index a8759df0eed..9af43c8d6b1 100644 --- a/src/everest/simulator/everest_to_ert.py +++ b/src/everest/simulator/everest_to_ert.py @@ -411,12 +411,12 @@ def _extract_forward_model(ever_config: EverestConfig, ert_config): forward_model += _extract_templating(ever_config) forward_model += ever_config.forward_model or [] - sim_job = ert_config.get(ErtConfigKeys.SIMULATION_JOB, []) + fm_steps = ert_config.get(ErtConfigKeys.FORWARD_MODEL, []) for job in forward_model: - tmp = job.split() - sim_job.append(tuple(tmp)) + job_name, *args = job.split() + fm_steps.append([job_name, args]) - ert_config[ErtConfigKeys.SIMULATION_JOB] = sim_job + ert_config[ErtConfigKeys.FORWARD_MODEL] = fm_steps def _extract_model(ever_config: EverestConfig, ert_config): diff --git a/test-data/ert/heat_equation/HEAT_EQUATION b/test-data/ert/heat_equation/HEAT_EQUATION index 509bd67ce05..ffbe42c0de4 100644 --- a/test-data/ert/heat_equation/HEAT_EQUATION +++ b/test-data/ert/heat_equation/HEAT_EQUATION @@ -1 +1,2 @@ EXECUTABLE heat_equation.py +ARGLIST diff --git a/test-data/ert/heat_equation/config.ert b/test-data/ert/heat_equation/config.ert index 7d71daa6908..082123e932f 100644 --- a/test-data/ert/heat_equation/config.ert +++ b/test-data/ert/heat_equation/config.ert @@ -23,4 +23,4 @@ FIELD COND PARAMETER cond.bgrdecl INIT_FILES:cond.bgrdecl FORWARD_INIT:True GEN_DATA MY_RESPONSE RESULT_FILE:gen_data_%d.out REPORT_STEPS:10,71,132,193,255,316,377,438 INPUT_FORMAT:ASCII INSTALL_JOB heat_equation HEAT_EQUATION -SIMULATION_JOB heat_equation +FORWARD_MODEL heat_equation(=, =) diff --git a/test-data/ert/heat_equation/config_forward_init_false.ert b/test-data/ert/heat_equation/config_forward_init_false.ert index 8c3521c0805..549384a3b27 100644 --- a/test-data/ert/heat_equation/config_forward_init_false.ert +++ b/test-data/ert/heat_equation/config_forward_init_false.ert @@ -23,4 +23,4 @@ FIELD COND PARAMETER cond.bgrdecl INIT_FILES:cond_%d.bgrdecl FORWARD_INIT:False GEN_DATA MY_RESPONSE RESULT_FILE:gen_data_%d.out REPORT_STEPS:10,71,132,193,255,316,377,438 INPUT_FORMAT:ASCII INSTALL_JOB heat_equation HEAT_EQUATION -SIMULATION_JOB heat_equation +FORWARD_MODEL heat_equation(=, =) diff --git a/tests/ert/ui_tests/cli/test_local_driver.py b/tests/ert/ui_tests/cli/test_local_driver.py index 27726995f1a..b516e86047a 100644 --- a/tests/ert/ui_tests/cli/test_local_driver.py +++ b/tests/ert/ui_tests/cli/test_local_driver.py @@ -29,7 +29,7 @@ def create_ert_config(path: Path): NUM_REALIZATIONS 1 MIN_REALIZATIONS 1 INSTALL_JOB write_pid_to_file_and_sleep TEST_JOB - SIMULATION_JOB write_pid_to_file_and_sleep + FORWARD_MODEL write_pid_to_file_and_sleep """ ), encoding="utf-8", diff --git a/tests/ert/unit_tests/config/config_dict_generator.py b/tests/ert/unit_tests/config/config_dict_generator.py index 9acedff897d..e20fb8eb7e8 100644 --- a/tests/ert/unit_tests/config/config_dict_generator.py +++ b/tests/ert/unit_tests/config/config_dict_generator.py @@ -257,7 +257,6 @@ class ErtConfigValues: min_realizations: str define: list[tuple[str, str]] forward_model: tuple[str, list[tuple[str, str]]] - simulation_job: list[list[str]] stop_long_running: bool data_kw_key: list[tuple[str, str]] data_file: str @@ -292,7 +291,6 @@ class ErtConfigValues: def to_config_dict(self, config_file, cwd, all_defines=True): result = { ConfigKeys.FORWARD_MODEL: self.forward_model, - ConfigKeys.SIMULATION_JOB: self.simulation_job, ConfigKeys.NUM_REALIZATIONS: self.num_realizations, ConfigKeys.RUNPATH_FILE: self.runpath_file, ConfigKeys.RUN_TEMPLATE: self.run_template, @@ -387,7 +385,6 @@ def ert_config_values(draw, use_eclbase=booleans): queue_system = draw(queue_systems) install_jobs = draw(small_list(random_forward_model_names(words, file_names))) forward_model = draw(small_list(job(install_jobs))) if install_jobs else [] - simulation_job = draw(small_list(sim_job(install_jobs))) if install_jobs else [] gen_data = draw( small_list( st.tuples( @@ -436,7 +433,6 @@ def ert_config_values(draw, use_eclbase=booleans): st.builds( ErtConfigValues, forward_model=st.just(forward_model), - simulation_job=st.just(simulation_job), num_realizations=positives, eclbase=st.just(draw(words) + "%d") if use_eclbase else st.just(None), runpath_file=st.just(draw(file_names) + "runpath"), @@ -715,11 +711,6 @@ def to_config_file(filename, config_values): if keyword in tuple_value_keywords: for tuple_key, tuple_value in keyword_value: config.write(f"{keyword} {tuple_key} {tuple_value}\n") - elif keyword == ConfigKeys.SIMULATION_JOB: - for job_config in keyword_value: - job_name = job_config[0] - job_args = " ".join(job_config[1:]) - config.write(f"{keyword} {job_name} {job_args}\n") elif keyword == ConfigKeys.FORWARD_MODEL: for job_name, job_args in keyword_value: config.write( diff --git a/tests/ert/unit_tests/config/test_create_forward_model_json.py b/tests/ert/unit_tests/config/test_create_forward_model_json.py index 2f5846038fc..43a87f3c7be 100644 --- a/tests/ert/unit_tests/config/test_create_forward_model_json.py +++ b/tests/ert/unit_tests/config/test_create_forward_model_json.py @@ -599,81 +599,6 @@ def test_that_config_path_is_the_directory_of_the_main_ert_config(): assert data["jobList"][0]["argList"] == [os.getcwd()] -@pytest.mark.usefixtures("use_tmpdir") -@pytest.mark.parametrize( - "job, forward_model, expected_args", - [ - pytest.param( - dedent( - """ - EXECUTABLE echo - MIN_ARG 1 - MAX_ARG 6 - ARG_TYPE 0 STRING - ARG_TYPE 1 BOOL - ARG_TYPE 2 FLOAT - ARG_TYPE 3 INT - """ - ), - "SIMULATION_JOB job_name Hello True 3.14 4", - ["Hello", "True", "3.14", "4"], - id="Not all args", - ), - pytest.param( - dedent( - """ - EXECUTABLE echo - MIN_ARG 1 - MAX_ARG 2 - ARG_TYPE 0 STRING - ARG_TYPE 0 STRING - """ - ), - "SIMULATION_JOB job_name word ", - ["word", ""], - id="Some args", - ), - pytest.param( - dedent( - """ - EXECUTABLE echo - MIN_ARG 1 - MAX_ARG 2 - ARG_TYPE 0 STRING - ARG_TYPE 0 STRING - ARGLIST - """ - ), - "SIMULATION_JOB job_name arga argb", - ["arga", "argb"], - id="simulation job with arglist", - ), - ], -) -def test_simulation_job(job, forward_model, expected_args): - with open("job_file", "w", encoding="utf-8") as fout: - fout.write(job) - - with open("config_file.ert", "w", encoding="utf-8") as fout: - # Write a minimal config file - fout.write("NUM_REALIZATIONS 1\n") - fout.write("INSTALL_JOB job_name job_file\n") - fout.write(forward_model) - - ert_config = ErtConfig.from_file("config_file.ert") - data = create_forward_model_json( - context=ert_config.substitutions, - forward_model_steps=ert_config.forward_model_steps, - env_vars=ert_config.env_vars, - user_config_file=ert_config.user_config_file, - run_id="", - ) - - assert len(ert_config.forward_model_steps) == 1 - fm_data = data["jobList"][0] - assert fm_data["argList"] == expected_args - - @pytest.mark.usefixtures("use_tmpdir") def test_that_private_over_global_args_gives_logging_message(caplog): caplog.set_level(logging.INFO) diff --git a/tests/ert/unit_tests/config/test_ert_config.py b/tests/ert/unit_tests/config/test_ert_config.py index 2438849e03f..6b18344562f 100644 --- a/tests/ert/unit_tests/config/test_ert_config.py +++ b/tests/ert/unit_tests/config/test_ert_config.py @@ -775,17 +775,16 @@ def run(self, *args): assert ert_config.workflows["workflow"].cmd_list[0][1] == ["0"] -@pytest.mark.parametrize("fm_keyword", ["FORWARD_MODEL", "SIMULATION_JOB"]) -def test_that_unknown_job_gives_config_validation_error(fm_keyword): +def test_that_unknown_job_gives_config_validation_error(): with pytest.raises( ConfigValidationError, match="Could not find forward model step 'NO_SUCH_FORWARD_MODEL_STEP'", ): _ = ErtConfig.from_file_contents( dedent( - f""" + """ NUM_REALIZATIONS 1 - {fm_keyword} NO_SUCH_FORWARD_MODEL_STEP + FORWARD_MODEL NO_SUCH_FORWARD_MODEL_STEP """ ) ) @@ -1847,11 +1846,6 @@ def test_warning_raised_when_summary_key_and_no_simulation_job_present(): ("flow", "FORWARD_MODEL"), ("FLOW", "FORWARD_MODEL"), ("ECLIPSE100", "FORWARD_MODEL"), - ("eclipse", "SIMULATION_JOB"), - ("eclipse100", "SIMULATION_JOB"), - ("flow", "SIMULATION_JOB"), - ("FLOW", "SIMULATION_JOB"), - ("ECLIPSE100", "SIMULATION_JOB"), ], ) @pytest.mark.usefixtures("use_tmpdir") diff --git a/tests/everest/test_detached.py b/tests/everest/test_detached.py index a3b9f660a1b..68312cdb644 100644 --- a/tests/everest/test_detached.py +++ b/tests/everest/test_detached.py @@ -171,7 +171,7 @@ def _get_reference_config(): DETACHED_NODE_DIR, SIMULATION_DIR, ), - "SIMULATION_JOB": [ + "FORWARD_MODEL": [ [ EVEREST_SERVER_CONFIG, "--config-file", diff --git a/tests/everest/test_egg_simulation.py b/tests/everest/test_egg_simulation.py index 66412d9c9c4..f0bad9f93ed 100644 --- a/tests/everest/test_egg_simulation.py +++ b/tests/everest/test_egg_simulation.py @@ -481,61 +481,68 @@ def _generate_exp_ert_config(config_path, output_dir): os.path.realpath("everest/model"), "everest_output/.res_runpath_list", ), - ErtConfigKeys.SIMULATION_JOB: [ - ( + ErtConfigKeys.FORWARD_MODEL: [ + [ "copy_directory", - f"{config_path}/../../eclipse/include/" - "realizations/realization-/eclipse", - "eclipse", - ), - ( + [ + f"{config_path}/../../eclipse/include/" + "realizations/realization-/eclipse", + "eclipse", + ], + ], + [ "symlink", - f"{config_path}/../input/files", - "files", - ), - ( + [f"{config_path}/../input/files", "files"], + ], + [ "copy_file", - os.path.realpath( - "everest/model/everest_output/.internal_data/wells.json" - ), - "wells.json", - ), - ( + [ + os.path.realpath( + "everest/model/everest_output/.internal_data/wells.json" + ), + "wells.json", + ], + ], + [ "well_constraints", - "-i", - "files/well_readydate.json", - "-c", - "files/wc_config.yml", - "-rc", - "well_rate.json", - "-o", - "wc_wells.json", - ), - ( + [ + "-i", + "files/well_readydate.json", + "-c", + "files/wc_config.yml", + "-rc", + "well_rate.json", + "-o", + "wc_wells.json", + ], + ], + [ "add_templates", - "-i", - "wc_wells.json", - "-c", - "files/at_config.yml", - "-o", - "at_wells.json", - ), - ( + [ + "-i", + "wc_wells.json", + "-c", + "files/at_config.yml", + "-o", + "at_wells.json", + ], + ], + [ "schmerge", - "-s", - "eclipse/include/schedule/schedule.tmpl", - "-i", - "at_wells.json", - "-o", - "eclipse/include/schedule/schedule.sch", - ), - ( + [ + "-s", + "eclipse/include/schedule/schedule.tmpl", + "-i", + "at_wells.json", + "-o", + "eclipse/include/schedule/schedule.sch", + ], + ], + [ "eclipse100", - "eclipse/model/EGG.DATA", - "--version", - "2020.2", - ), - ("rf", "-s", "eclipse/model/EGG", "-o", "rf"), + ["eclipse/model/EGG.DATA", "--version", "2020.2"], + ], + ["rf", ["-s", "eclipse/model/EGG", "-o", "rf"]], ], ErtConfigKeys.ENSPATH: os.path.join( os.path.realpath("everest/model"), diff --git a/tests/everest/test_res_initialization.py b/tests/everest/test_res_initialization.py index bc7e989971d..40800a8cf4d 100644 --- a/tests/everest/test_res_initialization.py +++ b/tests/everest/test_res_initialization.py @@ -202,7 +202,7 @@ def test_install_data_no_init(tmp_path, source, target, symlink, cmd, monkeypatc ert_config = everest_to_ert_config(ever_config) expected_fm = next(val for val in ert_config.forward_model_steps if val.name == cmd) - assert expected_fm.arglist == (f"./{source}", target) + assert expected_fm.arglist == [f"./{source}", target] @skipif_no_opm