diff --git a/src/ert/callbacks.py b/src/ert/callbacks.py index 4c59053644b..9b77f30ef02 100644 --- a/src/ert/callbacks.py +++ b/src/ert/callbacks.py @@ -6,7 +6,7 @@ from pathlib import Path from typing import Iterable -from ert.config import ParameterConfig, ResponseConfig +from ert.config import InvalidResponseFile, ParameterConfig, ResponseConfig from ert.run_arg import RunArg from ert.storage.realization_storage_state import RealizationStorageState @@ -55,6 +55,12 @@ async def _write_responses_to_storage( start_time = time.perf_counter() logger.debug(f"Starting to load response: {config.name}") ds = config.read_from_file(run_arg.runpath, run_arg.iens) + try: + ds = config.read_from_file(run_arg.runpath, run_arg.iens) + except (FileNotFoundError, InvalidResponseFile) as err: + errors.append(str(err)) + logger.warning(f"Failed to write: {run_arg.iens}: {err}") + continue await asyncio.sleep(0) logger.debug( f"Loaded {config.name}", @@ -67,8 +73,14 @@ async def _write_responses_to_storage( f"Saved {config.name} to storage", extra={"Time": f"{(time.perf_counter() - start_time):.4f}s"}, ) - except ValueError as err: + except Exception as err: errors.append(str(err)) + logger.exception( + f"Unexpected exception while writing response to storage {run_arg.iens}", + exc_info=err, + ) + continue + if errors: return LoadResult(LoadStatus.LOAD_FAILURE, "\n".join(errors)) return LoadResult(LoadStatus.LOAD_SUCCESSFUL, "") @@ -95,7 +107,10 @@ async def forward_model_ok( ) except Exception as err: - logging.exception(f"Failed to load results for realization {run_arg.iens}") + logger.exception( + f"Failed to load results for realization {run_arg.iens}", + exc_info=err, + ) parameters_result = LoadResult( LoadStatus.LOAD_FAILURE, "Failed to load results for realization " diff --git a/src/ert/config/__init__.py b/src/ert/config/__init__.py index f4585bee2a0..4c43de6ac52 100644 --- a/src/ert/config/__init__.py +++ b/src/ert/config/__init__.py @@ -31,7 +31,7 @@ WarningInfo, ) from .queue_config import QueueConfig -from .response_config import ResponseConfig +from .response_config import InvalidResponseFile, ResponseConfig from .summary_config import SummaryConfig from .summary_observation import SummaryObservation from .surface_config import SurfaceConfig @@ -66,6 +66,7 @@ "GenKwConfig", "HookRuntime", "IESSettings", + "InvalidResponseFile", "ModelConfig", "ParameterConfig", "PriorDict", diff --git a/src/ert/config/_read_summary.py b/src/ert/config/_read_summary.py index 55639b7259d..d8a9c9af438 100644 --- a/src/ert/config/_read_summary.py +++ b/src/ert/config/_read_summary.py @@ -92,6 +92,9 @@ def from_keyword(cls, summary_keyword: str) -> _SummaryType: return cls.OTHER +from .response_config import InvalidResponseFile + + def _cell_index( array_index: int, nx: PositiveInt, ny: PositiveInt ) -> Tuple[int, int, int]: @@ -110,7 +113,7 @@ def _check_if_missing( keyword_name: str, missing_key: str, *test_vars: Optional[T] ) -> List[T]: if any(v is None for v in test_vars): - raise ValueError( + raise InvalidResponseFile( f"Found {keyword_name} keyword in summary " f"specification without {missing_key} keyword" ) @@ -128,7 +131,13 @@ def make_summary_key( lj: Optional[int] = None, lk: Optional[int] = None, ) -> Optional[str]: - sum_type = _SummaryType.from_keyword(keyword) + try: + sum_type = _SummaryType.from_keyword(keyword) + except Exception as err: + raise InvalidResponseFile( + f"Could not read summary keyword '{keyword}': {err}" + ) from err + if sum_type in [ _SummaryType.FIELD, _SummaryType.OTHER, @@ -177,7 +186,7 @@ def make_summary_key( if sum_type == _SummaryType.NETWORK: (name,) = _check_if_missing("network", "WGNAMES", name) return f"{keyword}:{name}" - raise ValueError(f"Unexpected keyword type: {sum_type}") + raise InvalidResponseFile(f"Unexpected keyword type: {sum_type}") class DateUnit(Enum): @@ -189,7 +198,7 @@ def make_delta(self, val: float) -> timedelta: return timedelta(hours=val) if self == DateUnit.DAYS: return timedelta(days=val) - raise ValueError(f"Unknown date unit {val}") + raise InvalidResponseFile(f"Unknown date unit {val}") def _is_base_with_extension(base: str, path: str, exts: List[str]) -> bool: @@ -225,9 +234,9 @@ def _find_file_matching( dir, base = os.path.split(case) candidates = list(filter(lambda x: predicate(base, x), os.listdir(dir or "."))) if not candidates: - raise ValueError(f"Could not find any {kind} matching case path {case}") + raise FileNotFoundError(f"Could not find any {kind} matching case path {case}") if len(candidates) > 1: - raise ValueError( + raise FileNotFoundError( f"Ambiguous reference to {kind} in {case}, could be any of {candidates}" ) return os.path.join(dir, candidates[0]) @@ -254,7 +263,9 @@ def read_summary( summary, start_date, date_units, indices, date_index ) except resfo.ResfoParsingError as err: - raise ValueError(f"Failed to read summary file {filepath}: {err}") from err + raise InvalidResponseFile( + f"Failed to read summary file {filepath}: {err}" + ) from err return (start_date, keys, time_map, fetched) @@ -268,7 +279,7 @@ def _check_vals( kw: str, spec: str, vals: Union[npt.NDArray[Any], resfo.MESS] ) -> npt.NDArray[Any]: if vals is resfo.MESS or isinstance(vals, resfo.MESS): - raise ValueError(f"{kw.strip()} in {spec} has incorrect type MESS") + raise InvalidResponseFile(f"{kw.strip()} in {spec} has incorrect type MESS") return vals @@ -381,7 +392,7 @@ def _read_spec( # microsecond=self.micro_seconds % 10**6, ) except Exception as err: - raise ValueError( + raise InvalidResponseFile( f"SMSPEC {spec} contains invalid STARTDAT: {err}" ) from err keywords = arrays["KEYWORDS"] @@ -392,9 +403,9 @@ def _read_spec( lgr_names = arrays["LGRS "] if date is None: - raise ValueError(f"Keyword startdat missing in {spec}") + raise InvalidResponseFile(f"Keyword startdat missing in {spec}") if keywords is None: - raise ValueError(f"Keywords missing in {spec}") + raise InvalidResponseFile(f"Keywords missing in {spec}") if n is None: n = len(keywords) @@ -448,17 +459,17 @@ def optional_get(arr: Optional[npt.NDArray[Any]], idx: int) -> Any: units = arrays["UNITS "] if units is None: - raise ValueError(f"Keyword units missing in {spec}") + raise InvalidResponseFile(f"Keyword units missing in {spec}") if date_index is None: - raise ValueError(f"KEYWORDS did not contain TIME in {spec}") + raise InvalidResponseFile(f"KEYWORDS did not contain TIME in {spec}") if date_index >= len(units): - raise ValueError(f"Unit missing for TIME in {spec}") + raise InvalidResponseFile(f"Unit missing for TIME in {spec}") unit_key = _key2str(units[date_index]) try: date_unit = DateUnit[unit_key] except KeyError: - raise ValueError(f"Unknown date unit in {spec}: {unit_key}") from None + raise InvalidResponseFile(f"Unknown date unit in {spec}: {unit_key}") from None return ( date_index, diff --git a/src/ert/config/ert_script.py b/src/ert/config/ert_script.py index 2abb28cc3ab..419b7c5daa4 100644 --- a/src/ert/config/ert_script.py +++ b/src/ert/config/ert_script.py @@ -121,7 +121,7 @@ def initializeAndRun( arg_type = argument_types[index] if index < len(argument_types) else str if arg_value is not None: - arguments.append(arg_type(arg_value)) # type: ignore + arguments.append(arg_type(arg_value)) else: arguments.append(None) fixtures["workflow_args"] = arguments diff --git a/src/ert/config/gen_data_config.py b/src/ert/config/gen_data_config.py index cf4c4afe4d9..0dc10953a46 100644 --- a/src/ert/config/gen_data_config.py +++ b/src/ert/config/gen_data_config.py @@ -11,7 +11,7 @@ from ._option_dict import option_dict from .parsing import ConfigValidationError, ErrorInfo -from .response_config import ResponseConfig +from .response_config import InvalidResponseFile, ResponseConfig @dataclass @@ -71,12 +71,16 @@ def from_config_list(cls, gen_data: List[str]) -> Self: def read_from_file(self, run_path: str, _: int) -> xr.Dataset: def _read_file(filename: Path, report_step: int) -> xr.Dataset: - if not filename.exists(): - raise ValueError(f"Missing output file: {filename}") - data = np.loadtxt(_run_path / filename, ndmin=1) + try: + data = np.loadtxt(_run_path / filename, ndmin=1) + except ValueError as err: + raise InvalidResponseFile(str(err)) from err active_information_file = _run_path / (str(filename) + "_active") if active_information_file.exists(): - active_list = np.loadtxt(active_information_file) + try: + active_list = np.loadtxt(active_information_file) + except ValueError as err: + raise InvalidResponseFile(str(err)) from err data[active_list == 0] = np.nan return xr.Dataset( {"values": (["report_step", "index"], [data])}, @@ -93,15 +97,25 @@ def _read_file(filename: Path, report_step: int) -> xr.Dataset: if self.report_steps is None: try: datasets.append(_read_file(_run_path / filename_fmt, 0)) - except ValueError as err: - errors.append(str(err)) + except (InvalidResponseFile, FileNotFoundError) as err: + errors.append(err) else: for report_step in self.report_steps: filename = filename_fmt % report_step # noqa try: datasets.append(_read_file(_run_path / filename, report_step)) - except ValueError as err: - errors.append(str(err)) + except (InvalidResponseFile, FileNotFoundError) as err: + errors.append(err) if errors: - raise ValueError(f"Error reading GEN_DATA: {self.name}, errors: {errors}") + if all(isinstance(err, FileNotFoundError) for err in errors): + raise FileNotFoundError( + "Could not find one or more files/directories while reading GEN_DATA" + f" {self.name}: {','.join([str(err) for err in errors])}" + ) + else: + raise InvalidResponseFile( + "Error reading GEN_DATA " + f"{self.name}, errors: {','.join([str(err) for err in errors])}" + ) + return xr.combine_nested(datasets, concat_dim="report_step") diff --git a/src/ert/config/response_config.py b/src/ert/config/response_config.py index 97f27be0d0d..5f22c9c34ad 100644 --- a/src/ert/config/response_config.py +++ b/src/ert/config/response_config.py @@ -7,12 +7,27 @@ from ert.config.parameter_config import CustomDict +class InvalidResponseFile(Exception): + """ + Raised when an input file of the ResponseConfig has + the incorrect format. + """ + + @dataclasses.dataclass class ResponseConfig(ABC): name: str @abstractmethod - def read_from_file(self, run_path: str, iens: int) -> xr.Dataset: ... + def read_from_file(self, run_path: str, iens: int) -> xr.Dataset: + """Reads the data for the response from run_path. + + Raises: + FileNotFoundError: when one of the input_files for the + response is missing. + InvalidResponseFile: when one of the input_files is + invalid + """ def to_dict(self) -> Dict[str, Any]: data = dataclasses.asdict(self, dict_factory=CustomDict) diff --git a/src/ert/config/summary_config.py b/src/ert/config/summary_config.py index dfa2804f607..c71d173d1a4 100644 --- a/src/ert/config/summary_config.py +++ b/src/ert/config/summary_config.py @@ -8,7 +8,7 @@ import xarray as xr from ._read_summary import read_summary -from .response_config import ResponseConfig +from .response_config import InvalidResponseFile, ResponseConfig if TYPE_CHECKING: from typing import List @@ -37,7 +37,7 @@ def read_from_file(self, run_path: str, iens: int) -> xr.Dataset: # https://github.com/equinor/ert/issues/6974 # There is a bug with storing empty responses so we have # to raise an error in that case - raise ValueError( + raise InvalidResponseFile( f"Did not find any summary values matching {self.keys} in {filename}" ) ds = xr.Dataset( diff --git a/tests/unit_tests/config/test_gen_data_config.py b/tests/unit_tests/config/test_gen_data_config.py index 363db5c03d2..6362e8234c4 100644 --- a/tests/unit_tests/config/test_gen_data_config.py +++ b/tests/unit_tests/config/test_gen_data_config.py @@ -1,8 +1,13 @@ +import os +from contextlib import suppress +from pathlib import Path from typing import List +import hypothesis.strategies as st import pytest +from hypothesis import given -from ert.config import ConfigValidationError, GenDataConfig +from ert.config import ConfigValidationError, GenDataConfig, InvalidResponseFile @pytest.mark.parametrize( @@ -73,3 +78,57 @@ def test_malformed_or_missing_gen_data_result_file(result_file, error_message): GenDataConfig.from_config_list(config_line.split()) else: GenDataConfig.from_config_list(config_line.split()) + + +def test_that_invalid_gendata_outfile_error_propagates(tmp_path): + (tmp_path / "poly.out").write_text(""" + 4.910405046410615,4.910405046410615 + 6.562317389289953,6.562317389289953 + 9.599763191512997,9.599763191512997 + 14.022742453079745,14.022742453079745 + 19.831255173990197,19.831255173990197 + 27.025301354244355,27.025301354244355 + 35.604880993842215,35.604880993842215 + 45.56999409278378,45.56999409278378 + 56.92064065106905,56.92064065106905 + 69.65682066869802,69.65682066869802 + """) + + config = GenDataConfig( + name="gen_data", + input_file="poly.out", + ) + with pytest.raises( + InvalidResponseFile, + match="Error reading GEN_DATA.*could not convert string.*4.910405046410615,4.910405046410615.*to float64", + ): + config.read_from_file(tmp_path, 0) + + +@pytest.mark.usefixtures("use_tmpdir") +@given(st.binary()) +def test_that_read_file_does_not_raise_unexpected_exceptions_on_invalid_file(contents): + Path("./output").write_bytes(contents) + with suppress(InvalidResponseFile): + GenDataConfig( + name="gen_data", + input_file="output", + ).read_from_file(os.getcwd(), 0) + + +def test_that_read_file_does_not_raise_unexpected_exceptions_on_missing_file(tmpdir): + with pytest.raises(FileNotFoundError, match="DOES_NOT_EXIST not found"): + GenDataConfig( + name="gen_data", + input_file="DOES_NOT_EXIST", + ).read_from_file(tmpdir, 0) + + +def test_that_read_file_does_not_raise_unexpected_exceptions_on_missing_directory( + tmp_path, +): + with pytest.raises(FileNotFoundError, match="DOES_NOT_EXIST not found"): + GenDataConfig( + name="gen_data", + input_file="DOES_NOT_EXIST", + ).read_from_file(str(tmp_path / "DOES_NOT_EXIST"), 0) diff --git a/tests/unit_tests/config/test_read_summary.py b/tests/unit_tests/config/test_read_summary.py index 77c488c36b2..0af9fac55a5 100644 --- a/tests/unit_tests/config/test_read_summary.py +++ b/tests/unit_tests/config/test_read_summary.py @@ -7,6 +7,7 @@ from hypothesis import given from resdata.summary import Summary, SummaryVarType +from ert.config import InvalidResponseFile from ert.config._read_summary import _SummaryType, make_summary_key, read_summary from .summary_generator import ( @@ -277,7 +278,7 @@ def to_date(start_date: datetime, offset: float, unit: str) -> datetime: return start_date + timedelta(days=offset) if unit == "HOURS": return start_date + timedelta(hours=offset) - raise ValueError(f"Unknown time unit {unit}") + raise InvalidResponseFile(f"Unknown time unit {unit}") assert all( abs(actual - expected) <= timedelta(minutes=15) @@ -330,7 +331,7 @@ def test_that_incorrect_summary_files_raises_informative_errors( (tmp_path / "test.UNSMRY").write_bytes(smry_contents) (tmp_path / "test.SMSPEC").write_bytes(spec_contents) - with pytest.raises(ValueError, match=error_message): + with pytest.raises(InvalidResponseFile, match=error_message): read_summary(str(tmp_path / "test"), ["*"]) @@ -338,7 +339,7 @@ def test_mess_values_in_summary_files_raises_informative_errors(tmp_path): resfo.write(tmp_path / "test.SMSPEC", [("KEYWORDS", resfo.MESS)]) (tmp_path / "test.UNSMRY").write_bytes(b"") - with pytest.raises(ValueError, match="has incorrect type MESS"): + with pytest.raises(InvalidResponseFile, match="has incorrect type MESS"): read_summary(str(tmp_path / "test"), ["*"]) @@ -349,7 +350,7 @@ def test_empty_keywords_in_summary_files_raises_informative_errors(tmp_path): ) (tmp_path / "test.UNSMRY").write_bytes(b"") - with pytest.raises(ValueError, match="Got empty summary keyword"): + with pytest.raises(InvalidResponseFile, match="Got empty summary keyword"): read_summary(str(tmp_path / "test"), ["*"]) @@ -366,7 +367,7 @@ def test_missing_names_keywords_in_summary_files_raises_informative_errors( (tmp_path / "test.UNSMRY").write_bytes(b"") with pytest.raises( - ValueError, + InvalidResponseFile, match="Found block keyword in summary specification without dimens keyword", ): read_summary(str(tmp_path / "test"), ["*"]) @@ -386,7 +387,7 @@ def test_unknown_date_unit_in_summary_files_raises_informative_errors( (tmp_path / "test.UNSMRY").write_bytes(b"") with pytest.raises( - ValueError, + InvalidResponseFile, match="Unknown date unit .* ANNUAL", ): read_summary(str(tmp_path / "test"), ["*"]) @@ -405,7 +406,7 @@ def test_missing_units_in_summary_files_raises_an_informative_error( (tmp_path / "test.UNSMRY").write_bytes(b"") with pytest.raises( - ValueError, + InvalidResponseFile, match="Keyword units", ): read_summary(str(tmp_path / "test"), ["*"]) @@ -425,7 +426,7 @@ def test_missing_date_units_in_summary_files_raises_an_informative_error( (tmp_path / "test.UNSMRY").write_bytes(b"") with pytest.raises( - ValueError, + InvalidResponseFile, match="Unit missing for TIME", ): read_summary(str(tmp_path / "test"), ["*"]) @@ -445,7 +446,7 @@ def test_missing_time_keyword_in_summary_files_raises_an_informative_error( (tmp_path / "test.UNSMRY").write_bytes(b"") with pytest.raises( - ValueError, + InvalidResponseFile, match="KEYWORDS did not contain TIME", ): read_summary(str(tmp_path / "test"), ["*"]) @@ -464,7 +465,7 @@ def test_missing_keywords_in_smspec_raises_informative_error( (tmp_path / "test.UNSMRY").write_bytes(b"") with pytest.raises( - ValueError, + InvalidResponseFile, match="Keywords missing", ): read_summary(str(tmp_path / "test"), ["*"]) @@ -479,7 +480,7 @@ def test_that_ambiguous_case_restart_raises_an_informative_error( (tmp_path / "test.Smspec").write_bytes(b"") with pytest.raises( - ValueError, + FileNotFoundError, match="Ambiguous reference to unified summary", ): read_summary(str(tmp_path / "test"), ["*"]) diff --git a/tests/unit_tests/config/test_summary_config.py b/tests/unit_tests/config/test_summary_config.py index d04ab47bd2d..f0d45fef708 100644 --- a/tests/unit_tests/config/test_summary_config.py +++ b/tests/unit_tests/config/test_summary_config.py @@ -1,8 +1,17 @@ +import os +from contextlib import suppress +from pathlib import Path + import hypothesis.strategies as st import pytest from hypothesis import given -from ert.config import ConfigValidationError, ErtConfig, SummaryConfig +from ert.config import ( + ConfigValidationError, + ErtConfig, + InvalidResponseFile, + SummaryConfig, +) from .summary_generator import summaries @@ -23,7 +32,7 @@ def test_rading_empty_summaries_raises(wopr_summary): smspec, unsmry = wopr_summary smspec.to_file("CASE.SMSPEC") unsmry.to_file("CASE.UNSMRY") - with pytest.raises(ValueError, match="Did not find any summary values"): + with pytest.raises(InvalidResponseFile, match="Did not find any summary values"): SummaryConfig("summary", "CASE", ["WWCT:OP1"], None).read_from_file(".", 0) @@ -32,3 +41,29 @@ def test_summary_config_normalizes_list_of_keys(): "FOPR", "WOPR", ] + + +@pytest.mark.usefixtures("use_tmpdir") +@given(st.binary(), st.binary()) +def test_that_read_file_does_not_raise_unexpected_exceptions_on_invalid_file( + smspec, unsmry +): + Path("CASE.UNSMRY").write_bytes(unsmry) + Path("CASE.SMSPEC").write_bytes(smspec) + with suppress(InvalidResponseFile): + SummaryConfig("summary", ["CASE"], ["FOPR"]).read_from_file(os.getcwd(), 1) + + +def test_that_read_file_does_not_raise_unexpected_exceptions_on_missing_file(tmpdir): + with pytest.raises(FileNotFoundError): + SummaryConfig("summary", ["NOT_CASE"], ["FOPR"]).read_from_file(tmpdir, 1) + + +@pytest.mark.usefixtures("use_tmpdir") +def test_that_read_file_does_not_raise_unexpected_exceptions_on_missing_directory( + tmp_path, +): + with pytest.raises(FileNotFoundError): + SummaryConfig("summary", ["CASE"], ["FOPR"]).read_from_file( + str(tmp_path / "DOES_NOT_EXIST"), 1 + )