Reach EPANET link quantities via duplicated end breakpoints - #695
Open
jpalm3r wants to merge 2 commits into
Open
Reach EPANET link quantities via duplicated end breakpoints#695jpalm3r wants to merge 2 commits into
jpalm3r wants to merge 2 commits into
Conversation
jpalm3r
force-pushed
the
epanet-link-quantities
branch
from
August 10, 2026 13:25
460396c to
d842f9a
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the network/Res1D adapter pipeline so EPANET “link-node” reaches expose their link quantities (e.g. Flow/Velocity) via synthetic duplicated reach-end breakpoints, including support for merging reach-level .resx quantities onto those breakpoints. This aligns EPANET reach data access with the existing breakpoint-based find()/recall()/ReachObservation workflow used elsewhere in ModelSkill’s network subsystem.
Changes:
- Build reach breakpoints centrally (including “duplicate to both ends” behavior for short/synthetic-gridpoint reaches) and widen breakpoint distance typing to allow
Nonewhen the along-reach position is unknown. - Merge companion
.resxreach-level quantities onto reach breakpoints using the same “extra quantities” merge mechanism previously used for nodes. - Update EPANET-related docs and add/extend tests to validate breakpoint shape, graph edge lengths, and reach-observation extraction behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_network.py | Adds coverage for duplicated EPANET reach breakpoints, edge length semantics, reach-observation extraction, and _merge_extra_quantities error messaging. |
| src/modelskill/network.py | Allows ReachBreakPoint.id distances to be float | None and adds guards in graph-building / find() tolerance matching for unknown distances. |
| src/modelskill/model/network.py | Skips None-distance breakpoints during reach extraction to avoid calling find() with an unknown distance. |
| src/modelskill/model/adapters/_res1d.py | Moves reach breakpoint construction into _build_reach_breakpoints, adds reach-length resolution helper, and merges .resx reach quantities onto breakpoint data. |
| docs/user-guide/network.qmd | Updates EPANET network documentation to describe duplicated breakpoints and new reach-quantity reachability/limitations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
144
to
149
| * :attr:`id` - a ``(reach_id, distance)`` tuple that uniquely locates the | ||
| break point within the network. | ||
| break point within the network. ``distance`` may be ``None`` when the | ||
| break point's position along the reach is genuinely unknown (e.g. a | ||
| link-node reach with no known length). | ||
| * :attr:`data` - a time-indexed :class:`pandas.DataFrame` whose columns | ||
| are quantity names. |
Comment on lines
+241
to
+245
| # distance - find() treats distance=None the same as "not | ||
| # provided" and would raise. Such a breakpoint is unreachable | ||
| # this way, not missing; skip it rather than error, since | ||
| # another breakpoint on the same reach may still resolve. | ||
| if breakpoint.distance is None: |
jpalm3r
marked this pull request as ready for review
August 10, 2026 14:36
A link-node reach (EPANET) has one synthetic gridpoint belonging to neither end, so its own quantities — Flow on a pipe, energy on a pump — were unreachable through find() or ReachObservation. Duplicate that gridpoint into a breakpoint at each end: distance 0.0, and the reach length where known. Reaches with real gridpoints are unaffected. A companion .resx now contributes reach-level quantities the same way it already did for nodes, matched by gridpoint index, so _merge_extra_quantities takes a location_id rather than a node_id. Where the length is unknown the trailing breakpoint's distance is None, so ReachBreakPoint.distance widens to float | None and the graph's distance arithmetic guards against it. Such a breakpoint is not addressable by find(reach=..., distance=...), but stays reachable via ReachObservation and recall(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_resolve_alias walks the alias map comparing each key's distance against the requested one. A breakpoint with distance None — now possible on a link-node reach of unknown length — made that arithmetic raise a TypeError, even when a sibling breakpoint on the same reach would have resolved cleanly. Skip those keys instead: such a breakpoint is unreachable by distance, not missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jpalm3r
force-pushed
the
epanet-link-quantities
branch
from
August 11, 2026 09:54
98c3c98 to
0a4a007
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Stacked on #694 (
expose-reach-quantities) — merge that first. This diff will include #694's changes until then; the description below covers only what this branch adds on top of it.Summary
Fixes #680:
from_res1dcouldn't reach an EPANET link's own quantities (Flow,Velocity, ...), because a link-node reach has only one gridpoint and the existing gate (len(reach.gridpoints) > 2) excluded it from getting any breakpoints at all.0.0, and distancereach.lengthif known, otherwiseNone). Its quantities become reachable throughfind()/recall()/ReachObservation, the same way MIKE's reach-end data already is.ReachBreakPoint.distanceis widened tofloat | Noneto allow this, with guards added everywhere a breakpoint's distance was assumed to be a real number (_generate_graph's edge-length math,find()'s tolerance matching).Res1DReach.__init__into a standalone_build_reach_breakpoints, mirroring how node loading already resolves merged data before constructingRes1DNode. This keepsRes1DReachfree of Res1D/companion-file knowledge._build_reach_breakpointsalso merges the companion resx file's reach-level quantities (pump efficiency, energy, energy cost) onto each breakpoint, using the same_merge_extra_quantitiesmechanism node loading already uses._merge_extra_quantities'snode_idkeyword is renamed tolocation_idsince it now serves both nodes and reaches.from_epanetdocstring,network.qmd) updated to describe the new breakpoint shape and drop the closed-issue callout.Test plan
uv run pytest tests/passinguv run ruff check/uv run mypyon all touched files