From f7186b542fe8302b51b2666ec5520f9d0e16e841 Mon Sep 17 00:00:00 2001 From: "Yngve S. Kristiansen" Date: Wed, 19 Jul 2023 12:59:52 +0200 Subject: [PATCH] Raise error on faulty result file and missing report steps --- src/ert/_c_wrappers/enkf/ensemble_config.py | 26 ++-- src/ert/parsing/lark_parser.py | 3 +- .../test_parser_error_collection.py | 55 +++++++ .../res/enkf/test_ensemble_config.py | 139 ++++++++++++------ 4 files changed, 164 insertions(+), 59 deletions(-) diff --git a/src/ert/_c_wrappers/enkf/ensemble_config.py b/src/ert/_c_wrappers/enkf/ensemble_config.py index 7eafcf93942..433951479d3 100644 --- a/src/ert/_c_wrappers/enkf/ensemble_config.py +++ b/src/ert/_c_wrappers/enkf/ensemble_config.py @@ -254,21 +254,21 @@ def gen_data_node(gen_data: List[str]) -> Optional[GenDataConfig]: report_steps = rangestring_to_list(options.get(ConfigKeys.REPORT_STEPS, "")) if os.path.isabs(res_file) or "%d" not in res_file: - logger.error( - f"The RESULT_FILE:{res_file} setting for {name} is invalid - " - "must have an embedded %d - and be a relative path" - ) - elif not report_steps: - logger.error( - "The GEN_DATA keywords must have a REPORT_STEPS:xxxx defined" - "Several report steps separated with ',' and ranges with '-'" - "can be listed" + raise ConfigValidationError( + f"The RESULT_FILE:{res_file} setting for {name} is " + f"invalid - must have an embedded %d and be a relative path" ) - else: - gdc = GenDataConfig( - name=name, input_file=res_file, report_steps=report_steps + + if not report_steps: + raise ConfigValidationError( + "The GEN_DATA keywords must have " + "REPORT_STEPS:xxxx defined. Several " + "report steps separated with ',' " + "and ranges with '-' can be listed" ) - return gdc + + gdc = GenDataConfig(name=name, input_file=res_file, report_steps=report_steps) + return gdc @staticmethod def get_surface_node(surface: List[str]) -> SurfaceConfig: diff --git a/src/ert/parsing/lark_parser.py b/src/ert/parsing/lark_parser.py index 3b696657831..fd5c6e7a1f9 100644 --- a/src/ert/parsing/lark_parser.py +++ b/src/ert/parsing/lark_parser.py @@ -364,7 +364,8 @@ def _handle_includes( filename=config_file, ).set_context(error_context) ) - continue + + args = args[0:1] file_to_include = _substitute_token(defines, args[0]) diff --git a/tests/test_config_parsing/test_parser_error_collection.py b/tests/test_config_parsing/test_parser_error_collection.py index e820f60ce7b..2115f8a9d70 100644 --- a/tests/test_config_parsing/test_parser_error_collection.py +++ b/tests/test_config_parsing/test_parser_error_collection.py @@ -121,9 +121,17 @@ def assert_that_config_leads_to_error( with pytest.raises(ConfigValidationError) as caught_error: ErtConfig.from_file(config_filename, use_new_parser=True) + # If the ert config did not raise any errors + # we manually raise an "empty" error to make + # this raise an assertion error that can be + # acted upon from assert_that_config_does_not_lead_to_error + raise ConfigValidationError(errors=[]) collected_errors = caught_error.value.errors + if len(collected_errors) == 0: + raise AssertionError("Config did not lead to any errors") + # Find errors in matching file errors_matching_filename = find_and_assert_errors_matching_filename( errors=collected_errors, filename=expected_error.filename @@ -1059,3 +1067,50 @@ def test_that_deprecations_are_handled(contents, expected_errors): assert_that_config_leads_to_warning( config_file_contents=contents, expected_error=expected_error ) + + +@pytest.mark.parametrize("use_new_parser", [True, False]) +@pytest.mark.usefixtures("use_tmpdir") +def test_that_invalid_ensemble_result_file_errors(use_new_parser): + write_files( + { + "test.ert": """ +NUM_REALIZATIONS 1 +GEN_DATA RFT_3-1_R_DATA INPUT_FORMAT:ASCII REPORT_STEPS:100 RESULT_FILE:RFT_3-1_R_ + """ + } + ) + + with pytest.raises(ConfigValidationError, match="must have an embedded %d"): + _ = ErtConfig.from_file("test.ert", use_new_parser=use_new_parser) + + +@pytest.mark.parametrize("use_new_parser", [True, False]) +@pytest.mark.usefixtures("use_tmpdir") +def test_that_missing_report_steps_errors(use_new_parser): + write_files( + { + "test.ert": """ +NUM_REALIZATIONS 1 +GEN_DATA RFT_3-1_R_DATA INPUT_FORMAT:ASCII RESULT_FILE:RFT_3-1_R%d + + """ + } + ) + + with pytest.raises(ConfigValidationError, match="REPORT_STEPS"): + _ = ErtConfig.from_file("test.ert", use_new_parser=use_new_parser) + + +@pytest.mark.usefixtures("use_tmpdir") +def test_that_valid_gen_data_does_not_error(): + assert_that_config_does_not_lead_to_error( + config_file_contents=dedent( + """ +NUM_REALIZATIONS 1 +GEN_DATA RFT_3-1_R_DATA INPUT_FORMAT:ASCII REPORT_STEPS:100 RESULT_FILE:RFT_3-1_R%d + + """ + ), + unexpected_error=ExpectedErrorInfo(), + ) diff --git a/tests/unit_tests/c_wrappers/res/enkf/test_ensemble_config.py b/tests/unit_tests/c_wrappers/res/enkf/test_ensemble_config.py index 38283b8f55d..dcfb0f800e9 100644 --- a/tests/unit_tests/c_wrappers/res/enkf/test_ensemble_config.py +++ b/tests/unit_tests/c_wrappers/res/enkf/test_ensemble_config.py @@ -1,11 +1,12 @@ import os from datetime import datetime +from pathlib import Path import pytest +import xtgeo from ecl.summary import EclSum from ert._c_wrappers.enkf import ConfigKeys, EnsembleConfig, ErtConfig -from ert._c_wrappers.enkf.config.gen_data_config import GenDataConfig from ert.parsing import ConfigValidationError, ConfigWarning @@ -44,11 +45,16 @@ def test_ensemble_config_fails_on_non_sensical_refcase_file(): EnsembleConfig.from_dict(config_dict=config_dict) -def test_ensemble_config_construct_refcase_and_grid(setup_case): - setup_case("configuration_tests", "ensemble_config.ert") - grid_file = "grid/CASE.EGRID" - refcase_file = "input/refcase/SNAKE_OIL_FIELD" - +@pytest.mark.usefixtures("use_tmpdir") +def test_ensemble_config_construct_refcase_and_grid(): + grid_file = "CASE.EGRID" + refcase_file = "REFCASE_NAME" + xtgeo.create_box_grid(dimension=(10, 10, 1)).to_file("CASE.EGRID", "egrid") + ecl_sum = EclSum.writer("REFCASE_NAME", datetime(2014, 9, 10), 3, 3, 3) + ecl_sum.addVariable("FOPR", unit="SM3/DAY") + t_step = ecl_sum.addTStep(1, sim_days=10) + t_step["FOPR"] = 10 + ecl_sum.fwrite() ec = EnsembleConfig.from_dict( config_dict={ ConfigKeys.GRID: grid_file, @@ -80,6 +86,7 @@ def test_that_refcase_gets_correct_name(tmpdir): assert os.path.realpath(refcase_name) == ec.refcase.case +@pytest.mark.usefixtures("use_tmpdir") @pytest.mark.parametrize( "existing_suffix, expected_suffix", [ @@ -93,8 +100,7 @@ def test_that_refcase_gets_correct_name(tmpdir): ), ], ) -def test_that_files_for_refcase_exists(setup_case, existing_suffix, expected_suffix): - setup_case("configuration_tests", "ensemble_config.ert") +def test_that_files_for_refcase_exists(existing_suffix, expected_suffix): refcase_file = "missing_refcase_file" with open( @@ -113,45 +119,18 @@ def test_that_files_for_refcase_exists(setup_case, existing_suffix, expected_suf ) -@pytest.mark.parametrize( - "gen_data_str, expected", - [ - pytest.param( - "GDK RESULT_FILE:Results INPUT_FORMAT:ASCII REPORT_STEPS:10", - None, - id="RESULT_FILE missing %d in file name", - ), - pytest.param( - "GDK RESULT_FILE:Results%d INPUT_FORMAT:ASCII", - None, - id="REPORT_STEPS missing", - ), - pytest.param( - "GDK RESULT_FILE:Results%d INPUT_FORMAT:ASCII REPORT_STEPS:10,20,30", - "Valid", - id="Valid case", - ), - ], -) -def test_gen_data_node(gen_data_str, expected): - node = EnsembleConfig.gen_data_node(gen_data_str.split(" ")) - if expected is None: - assert node == expected - else: - assert node is not None - assert isinstance(node, GenDataConfig) - assert node.report_steps == [10, 20, 30] - - -def test_get_surface_node(setup_case): - _ = setup_case("configuration_tests", "ensemble_config.ert") +@pytest.mark.usefixtures("use_tmpdir") +def test_get_surface_node(): surface_str = "TOP" with pytest.raises(ConfigValidationError, match="Missing required OUTPUT_FILE"): EnsembleConfig.get_surface_node(surface_str.split(" ")) - surface_in = "surface/small_%d.irap" - base_surface = "surface/small.irap" - surface_out = "surface/small_out.irap" + surface_in = "small_%d.irap" + base_surface = "small.irap" + surface_out = "small_out.irap" + xtgeo.RegularSurface(ncol=2, nrow=3, xinc=1, yinc=1).to_file( + base_surface, fformat="irap_ascii" + ) # add init file surface_str += f" INIT_FILES:{surface_in}" @@ -175,7 +154,6 @@ def test_get_surface_node(setup_case): def test_surface_bad_init_values(setup_case): - _ = setup_case("configuration_tests", "ensemble_config.ert") surface_in = "path/surf.irap" base_surface = "path/not_surface" surface_out = "surface/small_out.irap" @@ -193,8 +171,9 @@ def test_surface_bad_init_values(setup_case): def test_ensemble_config_duplicate_node_names(setup_case): - _ = setup_case("configuration_tests", "ensemble_config.ert") duplicate_name = "Test_name" + Path("MULTFLT.TXT").write_text("", encoding="utf-8") + Path("FAULT_TEMPLATE").write_text("", encoding="utf-8") config_dict = { ConfigKeys.GEN_DATA: [ [ @@ -278,3 +257,73 @@ def test_gen_kw_pred_special_suggested_removal(): match="GEN_KW PRED used to hold a special meaning and be excluded", ): ErtConfig.from_file("config.ert") + + +@pytest.mark.usefixtures("use_tmpdir") +def test_gen_kw_config(): + with open("template.txt", "w", encoding="utf-8") as f: + f.write("Hello") + + with open("parameters.txt", "w", encoding="utf-8") as f: + f.write("KEY UNIFORM 0 1 \n") + + with open("parameters_with_comments.txt", "w", encoding="utf-8") as f: + f.write("KEY1 UNIFORM 0 1 -- COMMENT\n") + f.write("\n\n") # Two blank lines + f.write("KEY2 UNIFORM 0 1\n") + f.write("--KEY3 \n") + f.write("KEY3 UNIFORM 0 1\n") + + EnsembleConfig.from_dict( + config_dict={ + ConfigKeys.GEN_KW: [ + [ + "KEY", + "template.txt", + "nothing_here.txt", + "parameters.txt", + ] + ], + } + ) + + EnsembleConfig.from_dict( + config_dict={ + ConfigKeys.GEN_KW: [ + [ + "KEY", + "template.txt", + "nothing_here.txt", + "parameters_with_comments.txt", + ] + ], + } + ) + + with pytest.raises(ConfigValidationError): + EnsembleConfig.from_dict( + config_dict={ + ConfigKeys.GEN_KW: [ + [ + "KEY", + "no_template_here.txt", + "nothing_here.txt", + "parameters.txt", + ] + ], + } + ) + + with pytest.raises(ConfigValidationError): + EnsembleConfig.from_dict( + config_dict={ + ConfigKeys.GEN_KW: [ + [ + "KEY", + "template.txt", + "nothing_here.txt", + "no_parameter_here.txt", + ] + ], + } + )