Skip to content

Improve error message for invalid design matrix cells - #14123

Open
ajaust wants to merge 4 commits into
equinor:mainfrom
ajaust:improve-designmatrix-errors
Open

Improve error message for invalid design matrix cells#14123
ajaust wants to merge 4 commits into
equinor:mainfrom
ajaust:improve-designmatrix-errors

Conversation

@ajaust

@ajaust ajaust commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14108

Approach

Errors for empty or invalid design matrix cells reported a row index into
the already-filtered dataframe, so "Row 1" rarely matched the row the user
had to fix. Rows are now read without dropping blanks, filtered in-process,
and the originating spreadsheet row is carried into the message.

Both sheets anchor their read with skip_rows, which stops the reader from
trimming blank rows above the data; without it the numbers drift by the
number of leading blanks. A parametrised test pins this.

The message also no longer claims a cell is empty when it holds a rejected
word such as NONE, and the accepted values are documented.

(Screenshot of new behavior in GUI if applicable)

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

ajaust added 2 commits August 7, 2026 12:17
The list of disallowed contents was duplicated and could drift.
The row number in the Excel file typically differs from the internal row
number of the Polars dataframe since we remove empty lines, remove
headers for the design sheet, and 1-based counting within Excel.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request improves DESIGN_MATRIX validation feedback by reporting spreadsheet row numbers (rather than indices after internal filtering) and by clarifying that failures can be due to either empty cells or explicitly rejected cell values. It also documents the rejected values in the user-facing configuration reference and updates/extends unit tests to pin the new behavior (including handling of blank rows).

Changes:

  • Track original Excel row numbers while reading both design and default sheets, and use them in validation error messages.
  • Treat disallowed textual values (NONE/NULL/NAN, case-insensitive, whitespace-trimmed) as invalid and reflect that in the error wording.
  • Add/update documentation and unit tests to cover the new error messaging and row-number stability (including blank/leading blank rows).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/ert/unit_tests/sensitivity_analysis/test_design_matrix.py Updates expected error messages/row numbers and adds regression tests ensuring blank rows and leading blanks don’t shift reported rows.
src/ert/config/design_matrix.py Preserves spreadsheet row provenance via _drop_empty_rows, rewords validation errors, and centralizes disallowed cell values.
docs/ert/reference/configuration/keywords.rst Documents which design matrix cell values are considered invalid.
docs/ert/getting_started/howto/design_matrix.rst Updates the DESIGN_MATRIX validation description and links to the documented invalid cell values note.

@ajaust
ajaust force-pushed the improve-designmatrix-errors branch from 84b59f2 to a971127 Compare August 7, 2026 14:36
ajaust added 2 commits August 7, 2026 16:39
Cells with certain values are rejected by the DesignMatrix parser. The
old error message would always report an empty cell, even if it had an
disallowed value, which was confusing for users.
The rejected values were undocumented. A cell holding a word such as
NONE is visibly non-empty, so an error calling it empty or invalid is
hard to act on without a list of what is accepted.
@ajaust
ajaust force-pushed the improve-designmatrix-errors branch from a971127 to 33d9cd0 Compare August 7, 2026 14:41
@ajaust ajaust added this to SCOUT Aug 7, 2026
@ajaust
ajaust marked this pull request as ready for review August 7, 2026 14:43
@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.93%. Comparing base (31248f9) to head (33d9cd0).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14123      +/-   ##
==========================================
+ Coverage   91.85%   91.93%   +0.07%     
==========================================
  Files         484      482       -2     
  Lines       33581    33503      -78     
==========================================
- Hits        30847    30800      -47     
+ Misses       2734     2703      -31     
Flag Coverage Δ
cli-tests 36.25% <78.57%> (+0.07%) ⬆️
fuzz 44.22% <21.42%> (+0.13%) ⬆️
gui-tests 58.62% <78.57%> (+0.22%) ⬆️
performance-and-unit-tests 80.81% <100.00%> (+0.34%) ⬆️
test 46.04% <21.42%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/config/design_matrix.py 97.51% <100.00%> (+0.13%) ⬆️

... and 33 files with indirect coverage changes

@codspeed-hq

codspeed-hq Bot commented Aug 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing ajaust:improve-designmatrix-errors (33d9cd0) with main (40ae884)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

DESIGN_MATRIX: literal NONE, NaN or Null are treated as an empty cell

3 participants