Improve error message for invalid design matrix cells

docs/ert/getting_started/howto/design_matrix.rst

@@ -39,7 +39,8 @@ NOTE: The DESIGN_MATRIX validation is more strict than that of DESIGN2PARAMS.
 Some problems that were previously hidden or resulting in failures during runtime, will now result
 in errors during validation of the configuration:
 
-- Missing values (empty cells) in the design matrix
+- Missing or invalid values in the design matrix
+  (see :ref:`accepted cell values <design_matrix_cell_values>`)
 - Duplicate parameter names in the design matrix
 - Design sheet is empty
 - REAL column must only contain unique, positive integers

docs/ert/reference/configuration/keywords.rst

@@ -360,6 +360,13 @@ the final set of parameters (for example in parameters.txt in real==0) would be:
     as inactive in the ensemble and not run. For example, if the DESIGN_MATRIX only contains realization
     id 3, then the ensemble_size will be four. Here the realizations 0, 1, and 2 will be marked as inactive and not run.
 
+.. _design_matrix_cell_values:
+.. note::
+    Every cell in a used row or column of the design sheet and the default
+    sheet must hold a value. A cell is invalid if it is empty, contains only
+    whitespace, holds a numeric ``NaN``, or holds the text ``NONE``, ``NULL``
+    or ``NAN`` in any casing and ignoring surrounding whitespace.
+
 .. _eclbase:
 
 ECLBASE

src/ert/config/design_matrix.py

@@ -4,7 +4,7 @@
 from collections.abc import Callable
 from dataclasses import dataclass
 from pathlib import Path
-from typing import TYPE_CHECKING, cast
+from typing import TYPE_CHECKING, ClassVar, cast
 
 import numpy as np
 import polars as pl
@@ -31,6 +31,8 @@ class DesignMatrix:
     default_sheet: str | None
     priority_source: str = "design_matrix"
 
+    DISALLOWED_CELL_VALUES: ClassVar[list[str]] = ["nan", "null", "none", ""]
+
     def __post_init__(self) -> None:
         try:
             (
@@ -267,23 +269,27 @@ def read_and_validate_design_matrix(
             )
         except pl.exceptions.NoDataError as err:
             raise ValueError("Design sheet headers are empty.") from err
-        design_matrix_df = (
-            _read_excel(
-                lambda: pl.read_excel(
-                    self.xls_filename,
-                    sheet_name=self.design_sheet,
-                    has_header=False,
-                    drop_empty_cols=False,
-                    drop_empty_rows=True,
-                    raise_if_empty=False,
-                    infer_schema_length=None,
-                    read_options={"skip_rows": 1},
-                ),
-                f"Design sheet '{self.design_sheet}'",
-            )
-            .with_columns(pl.col(pl.Float32, pl.Float64).fill_nan(None))
-            .with_columns(pl.col(pl.String).str.strip_chars())
+        design_matrix_df = _read_excel(
+            lambda: pl.read_excel(
+                self.xls_filename,
+                sheet_name=self.design_sheet,
+                has_header=False,
+                drop_empty_cols=False,
+                drop_empty_rows=False,
+                raise_if_empty=False,
+                infer_schema_length=None,
+                read_options={"skip_rows": 1},
+            ),
+            f"Design sheet '{self.design_sheet}'",
+        )
+        # The header row is skipped while reading, so the first body row read
+        # corresponds to row 2 in the spreadsheet.
+        design_matrix_df, excel_row_numbers = _drop_empty_rows(
+            design_matrix_df, first_excel_row=2
         )
+        design_matrix_df = design_matrix_df.with_columns(
+            pl.col(pl.Float32, pl.Float64).fill_nan(None)
+        ).with_columns(pl.col(pl.String).str.strip_chars())
         if design_matrix_df.is_empty():
             raise ValueError("Design sheet body is empty.")
 
@@ -311,7 +317,9 @@ def read_and_validate_design_matrix(
         design_matrix_df = design_matrix_df.with_columns(
             [
                 pl.when(
-                    pl.col(col).str.to_lowercase().is_in(["nan", "null", "none", ""])
+                    pl.col(col)
+                    .str.to_lowercase()
+                    .is_in(DesignMatrix.DISALLOWED_CELL_VALUES)
                 )
                 .then(None)
                 .otherwise(pl.col(col))
@@ -332,7 +340,7 @@ def read_and_validate_design_matrix(
         param_names = tuple(param_names[i] for i in columns_to_keep)
 
         if errors := DesignMatrix._validate_design_matrix(
-            design_matrix_df, param_names
+            design_matrix_df, param_names, excel_row_numbers
         ):
             error_msg = "\n".join(errors)
             raise ValueError(f"Design matrix is not valid, error(s):\n{error_msg}")
@@ -392,11 +400,14 @@ def read_and_validate_design_matrix(
 
     @staticmethod
     def _validate_design_matrix(
-        design_matrix: pl.DataFrame, param_names: tuple[str]
+        design_matrix: pl.DataFrame,
+        param_names: tuple[str],
+        excel_row_numbers: list[int],
     ) -> list[str]:
         """
         Validate user inputted design matrix
-        :raises: ValueError if design matrix contains empty headers or empty cells
+        :raises: ValueError if design matrix contains empty headers, empty
+        cells, or cells with disallowed values
         """
         errors = []
         param_name_count = Counter(p for p in param_names if p is not None)
@@ -410,14 +421,17 @@ def _validate_design_matrix(
                 f" {duplicates_formatted}"
             )
         empties = [
-            f"Row {i}, column {param_names[j]}"
+            f"Row {excel_row_numbers[i]}, column {param_names[j]}"
             for i, j in zip(
                 *np.where(design_matrix.select(pl.all().is_null())),
                 strict=False,
             )
         ]
         if len(empties) > 0:
-            errors.append(f"Design matrix contains empty cells {empties}")
+            errors.append(
+                "Design matrix contains empty cells or cells with a "
+                f"disallowed value {empties}"
+            )
 
         for column_num, param_name in enumerate(param_names):
             if param_name is None or len(param_name.split()) == 0:
@@ -452,12 +466,18 @@ def _read_defaultssheet(
                 sheet_name=defaults_sheetname,
                 has_header=False,
                 drop_empty_cols=True,
-                drop_empty_rows=True,
+                drop_empty_rows=False,
                 raise_if_empty=False,
-                read_options={"dtypes": "string"},
+                # `skip_rows` anchors the read at an absolute spreadsheet row.
+                # Without it the reader trims blank rows above the first
+                # non-empty row, which would shift the reported row numbers.
+                read_options={"dtypes": "string", "skip_rows": 0},
             ),
             f"Default sheet '{defaults_sheetname}'",
         )
+        # The defaults sheet has no header row, so the first row read
+        # corresponds to row 1 in the spreadsheet.
+        default_df, excel_row_numbers = _drop_empty_rows(default_df, first_excel_row=1)
         if default_df.is_empty():
             return {}
         if len(default_df.columns) < 2:
@@ -468,7 +488,9 @@ def _read_defaultssheet(
         default_df = default_df.with_columns(
             [
                 pl.when(
-                    pl.col(col).str.to_lowercase().is_in(["nan", "null", "none", ""])
+                    pl.col(col)
+                    .str.to_lowercase()
+                    .is_in(DesignMatrix.DISALLOWED_CELL_VALUES)
                 )
                 .then(None)
                 .otherwise(pl.col(col))
@@ -477,13 +499,16 @@ def _read_defaultssheet(
             ]
         )
         empty_cells = [
-            f"Row {i}, column {j}"
+            f"Row {excel_row_numbers[i]}, column {j}"
             for i, j in zip(
                 *np.where(default_df.select(pl.all().is_null())), strict=False
             )
         ]
         if len(empty_cells) > 0:
-            raise ValueError(f"Default sheet contains empty cells {empty_cells}")
+            raise ValueError(
+                "Default sheet contains empty cells or cells with a "
+                f"disallowed value {empty_cells}"
+            )
         if default_df.select(pl.nth(0)).is_duplicated().any():
             raise ValueError("Default sheet contains duplicate parameter names")
 
@@ -494,6 +519,33 @@ def _read_defaultssheet(
         }
 
 
+def _drop_empty_rows(
+    df: pl.DataFrame, first_excel_row: int
+) -> tuple[pl.DataFrame, list[int]]:
+    """
+    Drop rows where every cell is empty, equivalently to polars'
+    ``drop_empty_rows`` read option, while keeping track of which spreadsheet
+    row each surviving row originated from.
+
+    Callers must anchor their read with the ``skip_rows`` read option, otherwise
+    the Excel reader trims blank rows above the first non-empty row and
+    ``first_excel_row`` no longer describes the first row of ``df``.
+
+    :param first_excel_row: spreadsheet row number of the first row in ``df``.
+    :returns: the filtered dataframe and the spreadsheet row number of each of
+        its rows.
+    """
+    if df.height == 0 or df.width == 0:
+        return df, []
+    keep_row = df.select(
+        ~pl.all_horizontal(pl.all().is_null()).alias("keep")
+    ).to_series()
+    excel_row_numbers = [
+        row_index + first_excel_row for row_index, keep in enumerate(keep_row) if keep
+    ]
+    return df.filter(keep_row), excel_row_numbers
+
+
 def _read_excel(
     read_excel: Callable[[], pl.DataFrame], sheet_description: str
 ) -> pl.DataFrame:

tests/ert/unit_tests/sensitivity_analysis/test_design_matrix.py

@@ -427,17 +427,20 @@ def test_reading_design_matrix_validate_headers(tmp_path, column_names, error_ms
     [
         pytest.param(
             [0, None, 1],
-            r"Design matrix contains empty cells \['Row 1, column a'\]",
+            r"Design matrix contains empty cells or cells with a disallowed value "
+            r"\['Row 3, column a'\]",
             id="duplicate entries",
         ),
         pytest.param(
             [0, "      ", 1],
-            r"Design matrix contains empty cells \['Row 1, column a'\]",
+            r"Design matrix contains empty cells or cells with a disallowed value "
+            r"\['Row 3, column a'\]",
             id="whitespace entries",
         ),
         pytest.param(
             [0, "some", np.nan],
-            r"Design matrix contains empty cells \['Row 2, column a'\]",
+            r"Design matrix contains empty cells or cells with a disallowed value "
+            r"\['Row 4, column a'\]",
             id="invalid float values",
         ),
     ],
@@ -470,17 +473,20 @@ def test_reading_design_matrix_validate_cells(tmp_path, values, error_msg):
         ),
         pytest.param(
             [["one", 1], ["b", ""], ["d", 6]],
-            r"Default sheet contains empty cells \['Row 1, column 1'\]",
+            r"Default sheet contains empty cells or cells with a disallowed value "
+            r"\['Row 2, column 1'\]",
             id="empty cells",
         ),
         pytest.param(
             [["something", 1], ["b", "          "], ["d", 6]],
-            r"Default sheet contains empty cells \['Row 1, column 1'\]",
+            r"Default sheet contains empty cells or cells with a disallowed value "
+            r"\['Row 2, column 1'\]",
             id="whitespace entries",
         ),
         pytest.param(
             [["something", 1], ["b", "None"], ["d", 6]],
-            r"Default sheet contains empty cells \['Row 1, column 1'\]",
+            r"Default sheet contains empty cells or cells with a disallowed value "
+            r"\['Row 2, column 1'\]",
             id="None entries",
         ),
         pytest.param(
@@ -658,6 +664,111 @@ def test_that_default_sheet_excel_error_cells_raise_config_validation_error(tmp_
 
     with pytest.raises(
         ConfigValidationError,
-        match=r"Default sheet contains empty cells",
+        match=r"Default sheet contains empty cells or cells with a disallowed value",
+    ):
+        DesignMatrix(design_path, "DesignSheet", "DefaultSheet")
+
+
+def _write_sheets(
+    design_path, design_rows, default_rows=(("dummy_default", 1),)
+) -> None:
+    """Write sheets cell by cell so that blank rows can be expressed as None."""
+    with Workbook(design_path) as xl_write:
+        for sheet_name, rows in (
+            ("DesignSheet", design_rows),
+            ("DefaultSheet", default_rows),
+        ):
+            worksheet = xl_write.add_worksheet(sheet_name)
+            for row_index, row in enumerate(rows):
+                for col_index, value in enumerate(row):
+                    if value is not None:
+                        worksheet.write(row_index, col_index, value)
+
+
+def test_that_blank_rows_do_not_shift_reported_design_sheet_row(tmp_path):
+    design_path = tmp_path / "design_matrix.xlsx"
+    _write_sheets(
+        design_path,
+        design_rows=[
+            ("REAL", "a", "b"),
+            (1, 0, 0),
+            (None, None, None),
+            (None, None, None),
+            (5, None, 2),  # spreadsheet row 5, empty cell in column a
+            (7, 1, 3),
+        ],
+    )
+
+    with pytest.raises(
+        ConfigValidationError,
+        match=r"Design matrix contains empty cells or cells with a disallowed value "
+        r"\['Row 5, column a'\]",
+    ):
+        DesignMatrix(design_path, "DesignSheet", "DefaultSheet")
+
+
+def test_that_blank_rows_do_not_shift_reported_default_sheet_row(tmp_path):
+    design_path = tmp_path / "design_matrix.xlsx"
+    _write_sheets(
+        design_path,
+        design_rows=[("REAL", "a"), (0, 1), (1, 2)],
+        default_rows=[
+            ("one", 1),
+            (None, None),
+            (None, None),
+            ("b", None),  # spreadsheet row 4, empty cell
+            ("d", 6),
+        ],
+    )
+
+    with pytest.raises(
+        ConfigValidationError,
+        match=r"Default sheet contains empty cells or cells with a disallowed value "
+        r"\['Row 4, column 1'\]",
+    ):
+        DesignMatrix(design_path, "DesignSheet", "DefaultSheet")
+
+
+def test_that_blank_rows_are_dropped_without_error(tmp_path):
+    design_path = tmp_path / "design_matrix.xlsx"
+    _write_sheets(
+        design_path,
+        design_rows=[
+            ("REAL", "a"),
+            (0, 1),
+            (None, None),
+            (1, 2),
+            (None, None),
+        ],
+        default_rows=[("one", 1), (None, None), ("d", 6)],
+    )
+
+    design_matrix = DesignMatrix(design_path, "DesignSheet", "DefaultSheet")
+
+    assert design_matrix.design_matrix_df["realization"].to_list() == [0, 1]
+    assert design_matrix.design_matrix_df["a"].to_list() == [1, 2]
+    assert design_matrix.design_matrix_df["one"].to_list() == [1, 1]
+
+
+@pytest.mark.parametrize("leading_blank_rows", [0, 1, 2, 3])
+def test_that_blank_rows_above_the_data_do_not_shift_reported_rows(
+    tmp_path, leading_blank_rows
+):
+    """Both sheets anchor their read with ``skip_rows``, so the reported row
+    numbers stay correct even when the data does not start in row 1.
+    """
+    design_path = tmp_path / "design_matrix.xlsx"
+    blank = [(None, None)] * leading_blank_rows
+    _write_sheets(
+        design_path,
+        design_rows=[("REAL", "a"), (0, 1), (1, 2)],
+        default_rows=[*blank, ("one", 1), ("b", None)],
+    )
+
+    bad_row = leading_blank_rows + 2
+    with pytest.raises(
+        ConfigValidationError,
+        match=r"Default sheet contains empty cells or cells with a disallowed value "
+        rf"\['Row {bad_row}, column 1'\]",
     ):
         DesignMatrix(design_path, "DesignSheet", "DefaultSheet")