Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
version: "1.8.27"

- name: Install dependencies
run: uv sync --group dev --group docs --group networks
run: uv sync --group dev --group docs --group network

- name: Build documentation
run: just docs
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/full_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
enable-cache: true

- name: Install dependencies
run: uv sync --group test --group networks --no-dev
run: uv sync --group test --group network --no-dev

- name: Install pandas 2.x
if: matrix.pandas-version == 'pandas2'
Expand All @@ -54,7 +54,7 @@ jobs:
- name: Test
run: just test

build-no-networks:
build-no-network:
runs-on: ubuntu-latest

steps:
Expand All @@ -68,7 +68,7 @@ jobs:
python-version: "3.12"
enable-cache: true

- name: Install dependencies (without networks)
- name: Install dependencies (without network)
run: uv sync --group test --no-dev

- name: Test
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/notebooks_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
python-version: "3.14"
enable-cache: true
- name: Install dependencies
run: uv sync --group test --group notebooks --group networks --no-dev
run: uv sync --group test --group notebooks --group network --no-dev
- name: Test notebooks
run: |
uv run pytest tests/notebooks/
5 changes: 4 additions & 1 deletion adr/010-optional-domain-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ Installation: `pip install modelskill modelskill-network`

**Open Questions:**
- Should `modelskill[all]` install all optional model types?
- How to handle version constraints for optional dependencies?
- How to handle version constraints for optional dependencies? Answered for network
support by [ADR-013](013-network-topology-in-mikeio1d.md): the `network` extra names a
minimum mikeio1d, because the topology layer ships there. Network support requires
whatever Python that release requires.
- Should optional dependencies be tested in CI for every commit or separately?

## Status Notes
Expand Down
12 changes: 11 additions & 1 deletion adr/012-network-format-constructors.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
# ADR-012: One Network Constructor per Modelling Product

**Status**: Draft
**Status**: Accepted, narrowed by [ADR-013](013-network-topology-in-mikeio1d.md)

**Date**: 2026-08

## Narrowed by ADR-013

The constructors, the companion arguments, the extension tables, the coverage test and the
`.inp` reader are mikeio1d's. It replaced `from_mike` and `from_epanet` with one
`Network.open` that reads the extension. Naming a constructor after the product that wrote
the file is still the rule, and mikeio1d applies it.

`NetworkModelResult` hands a path to mikeio1d. The refusal messages for `.out`, `.resx`
and the formats without a fixture are written there.

## Context

`Network` is built from result files read through mikeio1d, whose single `Res1D` class opens nine extensions across five products — MIKE 1D (`.res1d`), MIKE 11 (`.res11`), MOUSE (`.prf`, `.crf`, `.xrf`), EPANET (`.res`), SWMM (`.out`), Water Hammer (`.whr`), and `.resx`, which is shared by the last three. There is no per-format reader and no per-format constructor argument, so from mikeio1d's side all nine look alike. modelskill's constructor was named `from_res1d`, and its extension guard was briefly widened to accept everything mikeio1d could read — making the name promise one format while reading nine.
Expand Down
48 changes: 48 additions & 0 deletions adr/013-network-topology-in-mikeio1d.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# ADR-013: The Network Topology Layer Belongs to mikeio1d

**Status**: Accepted

**Date**: 2026-08

## Context

`modelskill.network` grew into a topology layer of its own: the abstract node/reach/breakpoint types, a `Res1D` adapter, one constructor per modelling product, the `.resx` and `.inp` companions, a table of which extensions we refuse and why, a networkx graph carrying reach lengths and boundary edges, an alias map, and `find`/`recall`/`to_dataset` on top. Roughly 630 code lines, against 25 for mikeio1d's `experimental.to_networkx`, which converts the same file and ignores gridpoints.

Most of those 630 lines do work the upstream function declines to do. The layer still sits on the wrong side of a line ADR-001 drew for mikeio: we call `mikeio.read()` and stop, without modelling dfsu geometry or policing its format list. Here we do both. `Res1D` reads nine extensions across five products; our tables decide which of the nine we accept, and a test fails our CI when a mikeio1d release adds a tenth. The fixtures those tables are checked against — `network.res1d`, `network_cali.res11`, `epanet.res/.resx/.inp` — are copies of mikeio1d's own. Meanwhile `NetworkModelResult` uses five members of `Network`, two of them private, and never traverses the graph.

## Decision

mikeio1d gains an optional network module that builds and owns `Network`. modelskill requires it and consumes what it produces.

| Owner | Pieces |
|---|---|
| mikeio1d | abstract types and `BasicNode`/`BasicReach`, the `Res1D` adapter, `Network.open`, the `.resx` and `.inp` companions, the extension policy tables, graph construction with its length and boundary semantics, the alias map, `find`, `recall`, `to_dataframe`, `to_dataset` |
| modelskill | `NetworkModelResult`, `NodeModelResult`, `NodeObservation`, `ReachObservation`, matching, the MIKE+ station resolver |

`NetworkModelResult` takes a `Network` the upstream module built, or a path it hands to that module. The module is an extra there, carrying networkx and xarray, so `to_dataset()` ships with the class. modelskill's `network` extra requires a mikeio1d release new enough to contain it.

Original IDs become the only identifier a user handles: `NodeObservation.at` takes a node name or a `(reach, distance)` pair, and no longer an integer. The alias integers stay an internal index. They exist because the ID space mixes names and break points, and a tuple cannot be an xarray coordinate value. A saved comparer records the original ID, with the integer beside it as `node_index`, so reloading does not depend on the numbering the installed mikeio1d handed out.

Before anything moved, the loader's output over six fixture loads was recorded: graph edges with their lengths and boundary flags, the alias map, the dataframe, and every answer `find` and `recall` give. Those snapshots are the upstream module's acceptance test.

Phase 1 landed as mikeio1d [#247](https://github.com/DHI/mikeio1d/pull/247), merged 2026-08-19. The snapshots pass there unchanged, twice: against the code moved verbatim, and again after the two product constructors collapsed into `Network.open`. The second pass covers the redesign.

modelskill 1.4.0 waits for the mikeio1d release carrying the module. That release is not out yet.

## Alternatives Considered

**Keep the layer here.** Defensible while the API is private. It means maintaining a format matrix, an EPANET `.inp` parser and a graph contract for traversals we never perform. It also means a CI failure whenever someone else's release adds a format.

**Move only the constructors and companions**, leaving the graph and the abstract types here. Splits the format knowledge from the topology it produces, and leaves `Res1DReach` here as the single adapter for a plug point with no second implementation.

**A separate `modelskill-network` package.** Rejected in ADR-010 for fragmenting the install. It would still own format knowledge that belongs with mikeio1d.

**Ask mikeio1d to guarantee stable node numbering** instead of dropping integers from our API. Puts a promise on someone else's release process, to protect a number users should not be handling.

## Consequences

- ADR-012 is narrowed: the constructors, the companion arguments, the extension tables and the coverage test become mikeio1d's. Naming a constructor after the product that wrote the file is still the rule, and mikeio1d applies it.
- ADR-010's open question about version constraints for optional dependencies is answered for this feature: the `network` extra pins a minimum mikeio1d, and network support requires whatever Python that release requires.
- A hand-built network needs mikeio1d installed, since `BasicNode`/`BasicReach` move too. That costs a .NET dependency for users who touch no MIKE file, which only matters for tests and for a backend nobody has written.
- Dropping `at=<int>` is a breaking change for a signature that shipped in the 1.4.0a3 alpha only, while the network module is opt-in and absent from the API reference. Removing it after 1.4.0 would cost more.
- Releases become coupled in one direction: a fix to network file reading ships on mikeio1d's schedule. A format mikeio1d adds no longer breaks our CI.
3 changes: 2 additions & 1 deletion adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ Each ADR follows this structure:
- [ADR-009](009-factory-pattern.md) - Factory pattern for type detection
- [ADR-010](010-optional-domain-dependencies.md) - Optional dependencies for domain-specific model types (Draft)
- [ADR-011](011-vertical-pre-extracted-columns.md) - VerticalModelResult ingests pre-extracted columns
- [ADR-012](012-network-format-constructors.md) - One Network constructor per modelling product (Draft)
- [ADR-012](012-network-format-constructors.md) - One Network constructor per modelling product (narrowed by ADR-013)
- [ADR-013](013-network-topology-in-mikeio1d.md) - The network topology layer belongs to mikeio1d

## Contributing

Expand Down
Binary file removed docs/images/res1d_network_mapping.png
Binary file not shown.
Loading
Loading