Add network topology validation at parse time

[ElyesAhmed]

• Validates source nodes are groups, paths end in fixed pressure, and no cycles
• New ParseContext::SCHEDULE_NETWORK_INVALID (THROW_EXCEPTION by default)
• Hooked into Schedule after each report step containing BRANPROP/NODEPROP
• Updated tests with lenient helper for legacy behaviour

[bska]

jenkins build this please

[Copilot]

Pull request overview

This PR introduces parse-time validation of extended network (BRANPROP/NODEPROP) topology in opm-common. Previously the network structure was only weakly validated (as noted in ExtNetwork.cpp), so inconsistent networks were silently accepted and only surfaced later in the simulator. The new Network::validateTopology helper checks, at the end of each report step that defines or redefines the network, that every source node (a node with no inlets) is a group, and that every flow path ends in a fixed-pressure node (also catching cycles). Problems are reported through the standard ParseContext/ErrorGuard protocol via a new SCHEDULE_NETWORK_INVALID category that defaults to throwing.

Changes:

• New NetworkValidation.{hpp,cpp} module implementing source-is-group, flow-path-termination, and cycle checks over ExtNetwork.
• New ParseContext::SCHEDULE_NETWORK_INVALID error category (default THROW_EXCEPTION), and a hook in Schedule::iterateScheduleSection that runs the check when a report step contains BRANPROP/NODEPROP.
• New Topology_Consistency test suite plus a lenient (IGNORE) helper, and two existing tests updated to expect throws for their intentionally-inconsistent decks.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
opm/input/eclipse/Schedule/Network/NetworkValidation.hpp	Declares validateTopology with detailed API docs.
opm/input/eclipse/Schedule/Network/NetworkValidation.cpp	Implements source/flow-path/cycle checks; skips detached nodes and standard (GRUPNET) networks.
opm/input/eclipse/Schedule/Schedule.cpp	Tracks first network keyword per report step and invokes validation after processing.
opm/input/eclipse/Parser/ParseContext.hpp	Declares the new SCHEDULE_NETWORK_INVALID category with documentation.
opm/input/eclipse/Parser/ParseContext.cpp	Defines and registers the new category (defaults to throwing).
CMakeLists_files.cmake	Adds NetworkValidation.cpp to the build.
tests/parser/NetworkTests.cpp	Adds topology-consistency tests and a lenient helper; updates two existing tests to expect throws.

The implementation is clean, well-documented, and well-tested; I found no functional defects. The one consideration I raised is that the new check defaults to THROW_EXCEPTION on the core parse path, which is a behavior change that could reject decks that previously parsed. Because that backward-compatibility impact is a meaningful design decision affecting downstream simulators, it merits human review.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ElyesAhmed]

Pull request overview

This PR introduces parse-time validation of extended network (BRANPROP/NODEPROP) topology in opm-common. Previously the network structure was only weakly validated (as noted in ExtNetwork.cpp), so inconsistent networks were silently accepted and only surfaced later in the simulator. The new Network::validateTopology helper checks, at the end of each report step that defines or redefines the network, that every source node (a node with no inlets) is a group, and that every flow path ends in a fixed-pressure node (also catching cycles). Problems are reported through the standard ParseContext/ErrorGuard protocol via a new SCHEDULE_NETWORK_INVALID category that defaults to throwing.

Changes:

* New `NetworkValidation.{hpp,cpp}` module implementing source-is-group, flow-path-termination, and cycle checks over `ExtNetwork`.

* New `ParseContext::SCHEDULE_NETWORK_INVALID` error category (default `THROW_EXCEPTION`), and a hook in `Schedule::iterateScheduleSection` that runs the check when a report step contains BRANPROP/NODEPROP.

* New `Topology_Consistency` test suite plus a lenient (IGNORE) helper, and two existing tests updated to expect throws for their intentionally-inconsistent decks.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File Description
opm/input/eclipse/Schedule/Network/NetworkValidation.hpp Declares validateTopology with detailed API docs.
opm/input/eclipse/Schedule/Network/NetworkValidation.cpp Implements source/flow-path/cycle checks; skips detached nodes and standard (GRUPNET) networks.
opm/input/eclipse/Schedule/Schedule.cpp Tracks first network keyword per report step and invokes validation after processing.
opm/input/eclipse/Parser/ParseContext.hpp Declares the new SCHEDULE_NETWORK_INVALID category with documentation.
opm/input/eclipse/Parser/ParseContext.cpp Defines and registers the new category (defaults to throwing).
CMakeLists_files.cmake Adds NetworkValidation.cpp to the build.
tests/parser/NetworkTests.cpp Adds topology-consistency tests and a lenient helper; updates two existing tests to expect throws.

The implementation is clean, well-documented, and well-tested; I found no functional defects. The one consideration I raised is that the new check defaults to THROW_EXCEPTION on the core parse path, which is a behavior change that could reject decks that previously parsed. Because that backward-compatibility impact is a meaningful design decision affecting downstream simulators, it merits human review.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

I have decided at least for the moment to keep THROW_EXCEPTION as the default error action for the following reasons:

1. Early Failure vs. Wasted Compute / Divergence:
   ◦ Invalid network topologies (e.g., source nodes without matching groups, unconnected flow paths, or cyclic connections) cannot be resolved or balanced by the simulator runtime.
   ◦ Allowing such decks past parse time leads to wasted computational resources and cryptic convergence or linear solver failures hours into a run. Failing at parse time provides immediate, actionable feedback.
2. Low-Friction Overrides for Legacy/Custom Workflows:
   ◦ Because this check uses the standard ParseContext / ErrorGuard mechanism, workflows requiring legacy lenient parsing can override the error action without code changes (e.g., setting SCHEDULE_NETWORK_INVALID to WARN or IGNORE via runtime options or parser configuration).
3. High Diagnostic Clarity:
   ◦ The error messages generated by NetworkValidation pinpoint the exact offending nodes and structural issues (e.g., identifying disconnected endpoints or cyclic loops), making deck fixes fast and straightforward.