diff --git a/.gcovr.cfg b/.gcovr.cfg deleted file mode 100644 index 9aa166e..0000000 --- a/.gcovr.cfg +++ /dev/null @@ -1,20 +0,0 @@ -[gcovr] -root = . - -# Object dirs for Python extension and standalone C++ test builds -object-directory = build -object-directory = build/cpp-tests-cov - -# Only include our sources -filter = include/netgraph -filter = src - -# Exclude tests and third-party deps -exclude = tests -exclude = bindings/.* -exclude-directories = .*/_deps/.* -exclude-directories = .*/venv/.* -exclude-directories = .*/site-packages/.* - -# Work around odd temp working dirs on macOS/pybind11 -gcov-ignore-errors = all diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9aa3aae..85d5023 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,6 +10,25 @@ on: workflow_dispatch: jobs: + # A tag push publishes whatever version pyproject.toml declares, so a mistyped + # tag would ship the wrong version silently. Fail fast instead. + check_version: + name: Tag matches project version + runs-on: ubuntu-latest + if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v') + steps: + - uses: actions/checkout@v4 + - name: Compare tag with pyproject version + run: | + python -c " + import tomllib, os, sys + version = tomllib.load(open('pyproject.toml','rb'))['project']['version'] + tag = os.environ['GITHUB_REF_NAME'].removeprefix('v') + if tag != version: + sys.exit(f'tag {tag!r} != pyproject version {version!r}') + print(f'tag matches project version: {version}') + " + build_wheels: name: Wheel (${{ matrix.os }}, ${{ matrix.cibw_archs }}) runs-on: ${{ matrix.os }} @@ -95,7 +114,7 @@ jobs: publish_pypi: name: Publish to PyPI - needs: [build_wheels, build_sdist, test_wheels_centos_stream_9] + needs: [check_version, build_wheels, build_sdist, test_wheels_centos_stream_9] runs-on: ubuntu-latest environment: pypi permissions: diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e625a9..78b6c81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,49 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.8.0] - 2026-08-24 + +### Added + +- **Flow Policy**: `set_static_paths(src, dst, paths)` pins a demand to explicit path bundles (MPLS-style routing), finishing a feature that was ported but never bound and whose port bound every flow to the first bundle. One flow per usable bundle, each bound to its own bundle in supply order; bundles are validated against the graph and pruned against the policy's masks, and a bundle with no surviving `src->dst` walk is down (no reroute). `EqualBalanced` spreads over the up bundles only. With static paths the policy neither grows its flow set nor reoptimizes; `max_path_cost`/`-factor`, `min_flow_count` and `reoptimize_flows_on_each_placement` are inert. +- **Shortest Paths**: `PredDAG.from_edges(graph, edges)` builds a single-path `PredDAG` from a contiguous edge-id sequence (`make_path_dag` in C++) -- the missing constructor for operator-defined explicit paths. Extracting it also deduplicates the two identical path-to-DAG conversion blocks in `k_shortest_paths` (outputs unchanged). + +### Fixed + +- **Max-Flow**: `calc_max_flow` could return less than the true maximum, since the tier loop augments only along forward SPF DAGs and never cancels an earlier placement (the reported `min_cut` then contradicted `total_flow`). Added a residual completion phase with reverse arcs. **This is the default configuration** of `max_flow`/`batch_max_flow` (`Proportional` + `require_capacity=True` + `shortest_path=False`), so recorded expected values, regression baselines and stored results must be regenerated: `total_flow` increases where the previous value was suboptimal, and `min_cut`, `edge_flows`, `residual_capacity` and `reachable_nodes` change with it. `sensitivity_analysis`, which is built on `calc_max_flow`, is affected too. +- **Shortest Paths**: Zero-cost edges produced a cyclic predecessor DAG, making `EqualBalanced` placement return 0 flow and `resolve_to_paths` hang. Equal-cost predecessors are now recorded only while a node is unsettled, and `resolve_to_paths` guards against cycles in caller-supplied DAGs. Side effect on graphs containing zero-cost edges: among nodes mutually reachable at equal distance the settle order now decides which alternatives are kept, so a `PredDAG` can hold fewer equal-cost predecessors than in 0.7.2 and ECMP fan-out narrows accordingly. Graphs with strictly positive costs are unaffected (verified bit-identical). +- **K-Shortest Paths**: Spur enumeration materialized every equal-cost path, exponential in ECMP fan-out (a 67-node ECMP ladder needed ~4 s and ~3.5 GiB for `k=3`, and a 73-node one ~19 s; both are now instant in ~34 MB). Enumeration is now bounded by the number of candidates still acceptable; tie-breaking among equal-cost paths may differ, but path counts and costs are unchanged. +- **Flow Policy**: `place_demand` silently routed a second `(src, dst)` pair over the first pair's paths, and neither `place_demand` nor `rebalance_demand` checked that the supplied `FlowGraph` wrapped the policy's own graph -- with equal edge counts that selected paths on one topology and placed flow on another. Both now raise `invalid_argument` (`ValueError` from Python); use one policy per demand, or call `remove_demand()` first to retarget it. +- **Graph Construction**: `from_arrays` now rejects a total edge cost at or above 2^62, which overflows the int64 path arithmetic in SPF and silently corrupts results. The bound is on the sum of all edge costs -- a conservative upper bound on any path -- so it can reject a graph whose individual paths could not actually overflow. +- **Flow State**: `compute_min_cut` and max-flow reachability derived placed flow from `capacity - residual`, overstating it for a `FlowState` built with a custom `residual_init`. Both now use `edge_flow`. +- **K-Shortest Paths**: `k_shortest_paths` read `paths.back()` on an empty vector when `max_cost_factor < 1.0` put the cost ceiling below the shortest path, crashing for `k > 1` (the `k == 1` early return masked it). All `k` now return no paths for such a factor. +- **Python Bindings**: `batch_max_flow` rejected a valid int32 `pairs` array on Windows, because the dtype check compared buffer format strings and NumPy spells int32 as `NPY_LONG` on LLP64 but `NPY_INT` on LP64. It now compares dtype equivalence, and rejects non-contiguous `pairs` rather than reading them as if packed. +- **Build**: `NETGRAPH_CORE_SANITIZE` passed its flags as one quoted string, so sanitizer builds never compiled, and the test target was missing them entirely. `make sanitize-test` also no longer sets `detect_leaks=1` on macOS, where it aborts. +- **Build**: Coverage builds now use `-fprofile-update=atomic` on GCC; the default non-atomic counters corrupt under the threads used by `batch_max_flow` and `sensitivity_analysis`. + +### Changed + +- **Max-Flow**: `FlowSummary.costs`/`flows` is now a cost-weighted breakdown of the total flow rather than a list of path costs. Entries added by the new completion phase are *marginal* costs -- the augmenting path's forward edge costs minus the cost of the flow it cancels -- so an entry need not correspond to any traversable `src->dst` path. Consumers that treat these as path costs (for example a cost-distribution report) should be reviewed. +- **Flow Policy**: `max_path_cost` no longer implies a relative factor of 1.0. Previously `max_path_cost_factor` defaulted to 1.0 whenever either limit was set, so setting `max_path_cost` alone also rejected every path costing more than the best path found -- making the absolute limit unreachable. The two limits are now independent, and a policy with only `max_path_cost` set admits every path up to that cost. (Found while fixing undefined behavior in the same expression: with an unreachable destination, `best_path_cost_` is the `INT64_MAX` sentinel and multiplying it by the factor overflowed the cast back to `int64`.) +- **Shortest Paths**: Replaced nested per-node predecessor vectors with flat intrusive lists, removing the allocation churn that dominated the hot path (~3-4x faster; output unchanged). +- **Flow Placement**: `place_on_dag` now rebuilds its `(parent, child)` edge groups into a reused arena instead of nested per-node vectors, which was ~70% of its runtime (~2-2.4x faster; output unchanged). +- **Max-Flow**: Parallelized `batch_max_flow` across source/destination pairs using `std::async`; workers claim pairs from a shared counter, so a batch whose costs are unevenly distributed still parallelizes. Thread count from `NGRAPH_CORE_BATCH_THREADS` env or hardware concurrency; set it to `1` when calling from your own worker pool to avoid oversubscription. +- **BREAKING** **C++ API**: `FlowPolicy::set_static_paths` changed signature from `(std::vector>)` to `(NodeId src, NodeId dst, std::vector bundles)`, and its behavior changed: the previous implementation handed every flow the first matching bundle instead of one bundle per flow. It was never bound to Python, so only C++ callers are affected. +- **Flow Policy**: `place_demand` computes one SPF when seeding initial flows instead of repeating an identical one per flow; seeding cost no longer scales with `min_flow_count`. +- **Flow Policy**: The `EqualBalanced` rebalance recursion (`place_demand` -> `rebalance_demand` -> `place_demand`) is now an iterative loop with numerically identical results (returned leftover may differ by ~1 ulp from floating-point summation order); with many pinned bundles of heterogeneous capacity the recursion depth grew like `U * ln(imbalance/kMinFlow)`, a stack-overflow risk on worker threads. +- **Python Bindings**: Three input errors now raise where they previously did not, or raise a different type -- adjust `except` clauses accordingly. `Algorithms.spf` raises `TypeError` (was `ValueError`) for a wrong `residual` length, matching every other length check; `batch_max_flow` raises `ValueError` for out-of-range node ids that previously produced a zero-flow summary absorbed silently into the batch; and `Algorithms.ksp` validates `dtype` up front, where an invalid value previously went unnoticed whenever the query returned no paths. +- **Python Bindings**: A wrong-typed `graph`/`algorithms` argument now raises `TypeError` instead of an opaque `RuntimeError: Unable to cast ... to C++ type '?'`. +- **Python Typing**: Corrected `_docs.py` (pybind11 enums and classes are not `enum.Enum`/`dataclass`, `MinCut.edges` is an `int32` array, removed a `Path` class that never existed, and added the constructors for `FlowIndex`, `FlowPolicyConfig` and the three enums) and widened the type-checking re-exports from 6 names to all 16, so `Algorithms`, `FlowPolicy`, `StrictMultiDiGraph` and friends are no longer `Unknown` to type checkers despite the shipped `py.typed`. A missing stub constructor is now worse than no stub, so a test asserts every stub constructor matches its binding. +- **Tests**: Several assertions could never fail and now do: a self-loop assertion sat inside `except Exception: pass`, a thread-safety test ended in `assert True` while its workers called `pytest.fail()` from a thread where it cannot fail the test, and two tautologies (`assert size >= 0`) were replaced with the properties actually under test. +- **Docs**: Corrected README/CONTRIBUTING (required CMake is 3.23 not 3.15, `make py-test` does not exist, `make test` does not collect coverage), the `sensitivity_analysis` description in README and `backend.hpp` (it measures flow *lost* on edge removal, not gain from relaxing capacity), the zero-copy and GIL claims, and header contracts for `from_arrays`, `PredDAG`, `FlowSummary.costs`, `calc_max_flow` and `FlowPolicy`. + +### Removed + +- **Dead code**: `.gcovr.cfg` (never read -- gcovr's default config name has no leading dot, and `make cov` passes every flag explicitly), the unreachable `FlowGraph` include and `expect_flow_conservation()` helper in the C++ test utilities, unused test helpers/fixtures, `_USE_MATH_DEFINES` (no `M_*` macro is used), and unused ``/``/``/``/`` includes across `src/` and `include/`. +- **BREAKING** **Python Bindings**: `Algorithms.build_graph_from_arrays`. Nothing used it (no test, and not NetGraph), and the `Graph` handle it returned could not be passed to `FlowGraph` or `FlowPolicy`, so it only ever served the stateless algorithms while looking like a peer of `build_graph`. Build the graph with `StrictMultiDiGraph.from_arrays()` and pass it to `Algorithms.build_graph()`. +- **BREAKING** **C++ API**: The `build_graph(std::shared_ptr)` overload on `Algorithms` and `Backend`, which existed solely to serve that binding. It was a pure virtual, so an out-of-tree `Backend` implementation must delete its now-dangling `override` to compile. Callers wanting the handle to own the graph construct it directly: `GraphHandle{my_shared_ptr}`. +- **BREAKING** **Python Bindings**: The `Flow` class (bound from `FlowRecord`). It was reachable only as `_netgraph_core.Flow`, never exported from `netgraph_core`, and no binding constructed or returned one, so nothing could obtain an instance; the name also collided with the C++ alias `using Flow = double`. + ## [0.7.2] - 2026-03-26 ### Fixed diff --git a/CMakeLists.txt b/CMakeLists.txt index 461f44e..a05aa3c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,7 +27,11 @@ add_executable(netgraph_core_tests target_link_libraries(netgraph_core_tests PRIVATE netgraph_core GTest::gtest_main) target_include_directories(netgraph_core_tests PRIVATE tests/cpp) if(NETGRAPH_CORE_COVERAGE AND (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")) + # Atomic counters: see the NETGRAPH_CORE_COVERAGE block below. target_compile_options(netgraph_core_tests PRIVATE -O0 -g --coverage) + if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + target_compile_options(netgraph_core_tests PRIVATE -fprofile-update=atomic) + endif() target_link_options(netgraph_core_tests PRIVATE --coverage) endif() include(GoogleTest) @@ -85,7 +89,6 @@ target_include_directories(netgraph_core PUBLIC $ $ ) -target_compile_definitions(netgraph_core PRIVATE _USE_MATH_DEFINES) ## C++ standard is set globally above. target_compile_features(netgraph_core PUBLIC cxx_std_20) @@ -206,8 +209,15 @@ endif() if(NETGRAPH_CORE_COVERAGE) message(STATUS "Enabling coverage instrumentation (disables optimizations)") if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") + # batch_max_flow and sensitivity_analysis run worker threads, and the default + # non-atomic profile counters corrupt under concurrent updates (gcov then + # emits impossible values such as "branch 4 taken -1"). + set(COV_FLAGS -O0 -g --coverage) + if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + list(APPEND COV_FLAGS -fprofile-update=atomic) + endif() foreach(tgt netgraph_core _netgraph_core) - target_compile_options(${tgt} PRIVATE -O0 -g --coverage) + target_compile_options(${tgt} PRIVATE ${COV_FLAGS}) target_link_options(${tgt} PRIVATE --coverage) endforeach() else() @@ -218,8 +228,16 @@ endif() if(NETGRAPH_CORE_SANITIZE) message(STATUS "Enabling sanitizers (disables optimizations)") if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") - set(SAN_FLAGS "-fsanitize=address,undefined -fno-omit-frame-pointer") - foreach(tgt netgraph_core _netgraph_core) + # Must be a CMake list: a single quoted string reaches the compiler as one + # argument and clang rejects "-fsanitize=address,undefined -fno-omit-frame-pointer". + set(SAN_FLAGS -fsanitize=address,undefined -fno-omit-frame-pointer) + set(SAN_TARGETS netgraph_core _netgraph_core) + if(TARGET netgraph_core_tests) + # The test executable links the sanitized static library, so it must be + # built and linked with the same flags to pull in the sanitizer runtime. + list(APPEND SAN_TARGETS netgraph_core_tests) + endif() + foreach(tgt ${SAN_TARGETS}) target_compile_options(${tgt} PRIVATE ${SAN_FLAGS}) target_link_options(${tgt} PRIVATE ${SAN_FLAGS}) endforeach() diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0fdc19e..6cd3577 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,7 +47,7 @@ Run specific test suites: ```bash make cpp-test # C++ tests (GoogleTest) -make py-test # Python tests (pytest) +make test # Python tests (pytest) ``` ### Code Style @@ -65,13 +65,19 @@ make format Releases are automated via GitHub Actions when a new tag is pushed. -1. Bump version in `pyproject.toml`. -2. Commit and push. -3. Create and push a tag: +1. Move the pending notes into a new `## [X.Y.Z] - YYYY-MM-DD` section in `CHANGELOG.md`, + dated the day you tag. Mark anything that breaks consumers as **BREAKING**. +2. Bump `version` in `pyproject.toml` to the same `X.Y.Z`. CI does not cross-check the + tag against this value, so a mismatch publishes the wrong version silently. +3. Commit and push. +4. Create and push a matching tag: ```bash - git tag v0.1.0 - git push origin v0.1.0 + git tag v0.8.0 + git push origin v0.8.0 ``` -4. The CI pipeline will build wheels, sdist, and publish to PyPI. +5. The CI pipeline builds wheels and the sdist, tests them, and publishes to PyPI via + Trusted Publishing. `make publish` / `make publish-test` upload whatever is in your + local `dist/` and are for Test PyPI or emergencies only -- real releases go through + the tag. diff --git a/Makefile b/Makefile index 0cab396..b10aa7d 100644 --- a/Makefile +++ b/Makefile @@ -52,7 +52,7 @@ help: @echo " make check-ci - Run non-mutating lint + tests (CI entrypoint)" @echo " make lint - Run only linting (non-mutating: ruff + pyright)" @echo " make format - Auto-format code with ruff" - @echo " make test - Run tests with coverage" + @echo " make test - Run Python tests (pytest)" @echo " make qt - Run quick tests only (exclude slow/benchmark)" @echo " make cpp-test - Build and run C++ tests" @echo " make cov - Coverage summary + XML + single-page combined HTML" @@ -281,6 +281,7 @@ cov: --object-directory build/cpp-tests-cov \ --filter 'include/netgraph' --filter 'src' --exclude 'tests' --exclude 'bindings/.*' --exclude '.*pybind11.*' --exclude '_deps/pybind11-src/.*' \ --gcov-ignore-errors=all \ + --gcov-ignore-parse-errors=negative_hits.warn_once_per_file \ --xml-pretty -o build/coverage/coverage-cpp.xml @echo "" @echo "================ Python + C++ coverage (summary) ================" @@ -296,7 +297,8 @@ sanitize-test: if command -v ninja >/dev/null 2>&1; then GEN_ARGS="-G Ninja"; fi; \ cmake -S . -B "$$BUILD_DIR" -DNETGRAPH_CORE_BUILD_TESTS=ON -DNETGRAPH_CORE_SANITIZE=ON -DCMAKE_BUILD_TYPE=Debug $$GEN_ARGS; \ cmake --build "$$BUILD_DIR" --config Debug -j; \ - ASAN_OPTIONS=detect_leaks=1 ctest --test-dir "$$BUILD_DIR" --output-on-failure || echo "⚠️ Some sanitizer tests failed" + if [ "$$(uname -s)" = "Darwin" ]; then ASAN_ENV="ASAN_OPTIONS=detect_leaks=0"; else ASAN_ENV="ASAN_OPTIONS=detect_leaks=1"; fi; \ + env $$ASAN_ENV ctest --test-dir "$$BUILD_DIR" --output-on-failure || echo "⚠️ Some sanitizer tests failed" # Clean + reinstall in dev mode (respects CMAKE_ARGS and MACOSX_DEPLOYMENT_TARGET) # Uses active PYTHON (venv or PATH) to avoid environment mismatches diff --git a/README.md b/README.md index 7ec5a36..c5d7fba 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ NetGraph-Core provides a specialized graph implementation for networking problem - **Determinism**: Guaranteed reproducible edge ordering by (cost, src, dst). - **Flow Modeling**: Native support for multi-commodity flow state, residual tracking, and ECMP/WCMP placement. -- **Performance**: Immutable CSR (Compressed Sparse Row) adjacency and zero-copy NumPy views. +- **Performance**: Immutable CSR (Compressed Sparse Row) adjacency, and zero-copy NumPy views over mutable flow state. ## Core Features @@ -28,13 +28,17 @@ NetGraph-Core provides a specialized graph implementation for networking problem - Yen's algorithm returning DAG-wrapped paths. - Configurable constraints on cost factors (e.g., paths within 1.5x of optimal). +- **Explicit Paths**: + - `PredDAG.from_edges(graph, edges)` builds a path bundle from an operator-supplied + edge sequence, usable anywhere a `PredDAG` is accepted. + - **Max-Flow**: - - **Algorithm**: Iterative augmentation using Successive Shortest Path on residual graphs, pushing flow across full ECMP/WCMP DAGs at each step. + - **Algorithm**: Iterative augmentation using Successive Shortest Path on residual graphs, pushing flow across full ECMP/WCMP DAGs at each step. For `Proportional` placement with `require_capacity=True`, a final residual completion phase augments with reverse arcs so the result is a true maximum flow; the other modes are placement models and may report less. - **Traffic Engineering (TE) Mode**: Routing adapts to residual capacity (progressive fill). - **IP Routing Mode**: Cost-only routing (ECMP/WCMP) ignoring capacity constraints. - **Analysis**: - - **Sensitivity Analysis**: Identifies bottleneck edges where capacity relaxation increases total flow. Supports `shortest_path` mode to analyze only edges used under ECMP routing (IP/IGP networks) vs. full max-flow (SDN/TE networks). + - **Sensitivity Analysis**: Identifies critical edges by removing each saturated edge and measuring how much total flow is *lost*. Supports `shortest_path` mode to analyze only edges used under ECMP routing (IP/IGP networks) vs. full max-flow (SDN/TE networks). - **Min-Cut**: Computes minimum cuts on residual graphs. ### 3. Flow Policy Engine @@ -46,11 +50,36 @@ Unified configuration object (`FlowPolicy`) that models diverse routing behavior - `EqualBalanced`: **ECMP** (equal splitting) - equal distribution across next-hops and parallel edges. - `Proportional`: **WCMP** (weighted splitting) - distribution proportional to residual capacity. - **Lifecycle Management**: Handles demand placement, re-optimization of existing flows, and constraints (path cost, stretch factor, flow counts). +- **Static (Pinned) Paths**: `FlowPolicy.set_static_paths()` pins a demand to explicit path bundles (MPLS-style). One flow per usable bundle; a bundle with no path surviving the failure masks is *down* and carries nothing, since a pinned path does not reroute. ### 4. Python Integration -- **Zero-Copy**: Exposes C++ internal buffers to Python as read-only NumPy arrays (float64/int64). -- **Concurrency**: Releases the Python GIL during graph algorithms to enable threading. +- **Zero-Copy**: `FlowState` and `FlowGraph` `*_view()` methods expose C++ buffers as read-only NumPy arrays that track mutation in place. `StrictMultiDiGraph.*_view()` returns copies instead, so the graph's immutability cannot be violated. +- **Concurrency**: Releases the Python GIL during the long-running algorithms (SPF, KSP, max-flow, placement) to enable threading. `PredDAG.resolve_to_paths` holds the GIL. `batch_max_flow` and `sensitivity_analysis` also use internal worker threads, sized by `NGRAPH_CORE_BATCH_THREADS` / `NGRAPH_CORE_SENSITIVITY_THREADS`. + +## Quick Start + +```python +import numpy as np +import netgraph_core as ngc + +# Two parallel paths from node 0 to node 3. +graph = ngc.StrictMultiDiGraph.from_arrays( + num_nodes=4, + src=np.array([0, 1, 0, 2], dtype=np.int32), + dst=np.array([1, 3, 2, 3], dtype=np.int32), + capacity=np.array([10.0, 10.0, 5.0, 5.0], dtype=np.float64), + cost=np.array([1, 1, 1, 1], dtype=np.int64), + ext_edge_ids=np.arange(4, dtype=np.int64), # your own stable edge ids +) + +algs = ngc.Algorithms(ngc.Backend.cpu()) +handle = algs.build_graph(graph) + +total, summary = algs.max_flow(handle, 0, 3) +print(total) # 15.0 +print(summary.min_cut.edges.tolist()) # [0, 1] -- bottleneck edge ids +``` ## Installation @@ -89,16 +118,24 @@ tests/py/ # Python tests (pytest) make dev # Setup: venv, dependencies, pre-commit hooks make check # Run all tests and linting (auto-fix formatting) make check-ci # Strict checks without auto-fix (for CI) -make test # Python tests with coverage +make test # Python tests make cpp-test # C++ tests only make cov # Combined coverage report (C++ + Python) ``` +## Environment Variables + +| Variable | Effect | +| --- | --- | +| `NGRAPH_CORE_PROFILE=1` | Enable profiling of C++ hot paths (`profiling_dump()` / `profiling_reset()`). | +| `NGRAPH_CORE_BATCH_THREADS` | Worker threads for `batch_max_flow` (default: hardware concurrency). Set to `1` when calling from your own worker pool. | +| `NGRAPH_CORE_SENSITIVITY_THREADS` | Worker threads for `sensitivity_analysis` (default: hardware concurrency). | + ## Requirements - **C++:** C++20 compiler (GCC 10+, Clang 12+, MSVC 2019+) - **Python:** 3.11+ -- **Build:** CMake 3.15+, scikit-build-core +- **Build:** CMake 3.23+, scikit-build-core - **Dependencies:** pybind11, NumPy ## License diff --git a/bindings/python/module.cpp b/bindings/python/module.cpp index 30249d1..1cde885 100644 --- a/bindings/python/module.cpp +++ b/bindings/python/module.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include "netgraph/core/k_shortest_paths.hpp" #include "netgraph/core/max_flow.hpp" @@ -155,38 +156,25 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { .def_static("cpu", [](){ return PyBackend{ make_cpu_backend() }; }); py::class_(m, "Graph") - // Opaque holder; constructed via Algorithms.build_graph or build_graph_from_arrays + // Opaque holder; constructed via Algorithms.build_graph .def_property_readonly("num_nodes", [](const PyGraph& pg){ return pg.num_nodes; }) .def_property_readonly("num_edges", [](const PyGraph& pg){ return pg.num_edges; }); py::class_>(m, "Algorithms") - .def(py::init([](const PyBackend& b){ return std::make_shared(b.impl); })) + .def(py::init([](const PyBackend& b){ return std::make_shared(b.impl); }), py::arg("backend")) .def("build_graph", [](const Algorithms& algs, py::object graph_obj){ + // graph_obj is py::object so PyGraph can keep it alive; check the type + // explicitly, since a bare .cast<> failure surfaces as an opaque + // "RuntimeError: Unable to cast ... to C++ type '?'". + if (!py::isinstance(graph_obj)) { + throw py::type_error("build_graph: graph must be a StrictMultiDiGraph"); + } const StrictMultiDiGraph& g = graph_obj.cast(); auto gh = algs.build_graph(g); // Keep the Python graph object alive alongside the handle when // referencing a non-owned graph instance. return PyGraph{ gh, graph_obj, g.num_nodes(), g.num_edges() }; }, py::arg("graph")) - .def("build_graph_from_arrays", [](const Algorithms& algs, - std::int32_t num_nodes, - py::array src, py::array dst, - py::array capacity, py::array cost, - py::array ext_edge_ids){ - // Build graph and construct shared ownership directly - auto ext_s = as_span(ext_edge_ids, "ext_edge_ids"); - auto sp = std::make_shared( - StrictMultiDiGraph::from_arrays( - num_nodes, - as_span(src, "src"), - as_span(dst, "dst"), - as_span(capacity, "capacity"), - as_span(cost, "cost"), - ext_s)); - auto gh = algs.build_graph(std::static_pointer_cast(sp)); - // GraphHandle holds shared ownership; no additional Python-side owner needed - return PyGraph{ gh, py::none(), sp->num_nodes(), sp->num_edges() }; - }, py::arg("num_nodes"), py::arg("src"), py::arg("dst"), py::arg("capacity"), py::arg("cost"), py::arg("ext_edge_ids")) .def("spf", [](const Algorithms& algs, const PyGraph& pg, std::int32_t src, py::object dst, py::object selection_obj, py::object residual_obj, py::object node_mask, py::object edge_mask, bool multipath, std::string dtype) -> py::tuple { @@ -199,6 +187,12 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { if (!(arr.flags() & py::array::c_style)) throw py::type_error("residual must be C-contiguous (np.ascontiguousarray)"); auto buf = arr.request(); if (buf.ndim != 1 || buf.format != py::format_descriptor::format()) throw py::type_error("residual must be 1-D float64"); + // Check length here so this matches the TypeError raised by every other + // length check (masks, FlowState residual); otherwise the core throws + // std::invalid_argument, which reaches Python as ValueError. + if (static_cast(buf.shape[0]) != pg.num_edges) { + throw py::type_error("residual length must equal " + std::to_string(pg.num_edges)); + } residual_vec.resize(static_cast(buf.shape[0])); std::memcpy(residual_vec.data(), buf.ptr, residual_vec.size()*sizeof(double)); opts.residual = std::span(residual_vec.data(), residual_vec.size()); @@ -228,6 +222,7 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { int k, py::object max_cost_factor, bool unique, py::object node_mask, py::object edge_mask, std::string dtype){ if (src < 0 || src >= pg.num_nodes || dst < 0 || dst >= pg.num_nodes) throw py::value_error("src/dst out of range"); if (k <= 0) throw py::value_error("k must be >= 1"); + if (dtype != "float64" && dtype != "int64") throw py::value_error("dtype must be 'float64' or 'int64'"); KspOptions opts; opts.k = k; opts.unique = unique; if (!max_cost_factor.is_none()) opts.max_cost_factor = py::cast(max_cost_factor); auto node_bs = to_bool_span_from_numpy(node_mask, static_cast(pg.num_nodes), "node_mask"); auto edge_bs = to_bool_span_from_numpy(edge_mask, static_cast(pg.num_edges), "edge_mask"); @@ -247,8 +242,6 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { auto* outp = dist_arr.mutable_data(); for (std::size_t i=0;i::infinity() : static_cast(dist[i]); out.append(py::make_tuple(std::move(dist_arr), pr.second)); - } else { - throw py::value_error("dtype must be 'float64' or 'int64'"); } } return out; @@ -267,13 +260,27 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { py::object node_masks, py::object edge_masks, FlowPlacement placement, bool shortest_path, bool require_capacity, bool with_edge_flows, bool with_reachable, bool with_residuals){ + // Check the dtype, not the buffer format string: NumPy spells int32 as + // NPY_LONG ('l') on LLP64 (Windows) and NPY_INT ('i') on LP64, while + // format_descriptor is always 'i'. Comparing format strings + // therefore rejects a genuine int32 array on Windows. isinstance uses + // dtype equivalence, matching as_span() above. + if (!py::isinstance>(pairs)) throw py::type_error("pairs dtype must be int32"); + if (!(pairs.flags() & py::array::c_style)) throw py::type_error("pairs must be C-contiguous (use np.ascontiguousarray)"); auto buf = pairs.request(); if (buf.ndim != 2 || buf.shape[1] != 2) throw py::type_error("pairs must be shape [B,2]"); - if (buf.format != py::format_descriptor::format()) throw py::type_error("pairs dtype must be int32"); const std::size_t B = static_cast(buf.shape[0]); std::vector> pp; pp.reserve(B); auto* p = static_cast(buf.ptr); for (std::size_t i=0;i= pg.num_nodes || + pp[i].second < 0 || pp[i].second >= pg.num_nodes) { + throw py::value_error("pairs[" + std::to_string(i) + "] has a src/dst out of range"); + } + } std::vector node_bufs, edge_bufs; std::vector> node_spans; std::vector> edge_spans; auto parse_mask_list = [&](py::object list_obj, std::size_t expected_len, const char* what, @@ -321,6 +328,16 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { .def_property_readonly("via_edges", [](const PredDAG& d){ return copy_to_numpy(std::span(d.via_edges.data(), d.via_edges.size())); }) + .def_static("from_edges", [](const StrictMultiDiGraph& g, py::sequence edges){ + std::vector e; + e.reserve(py::len(edges)); + for (auto item : edges) e.push_back(py::cast(item)); + return make_path_dag(g, e); + }, py::arg("graph"), py::arg("edges"), + "Build a single-path PredDAG from a contiguous edge-id sequence.\n" + "Validates that each edge exists, consecutive edges connect, and the walk " + "is a simple path. The result works anywhere a PredDAG is accepted " + "(place_on_dag, FlowGraph.place, FlowPolicy.set_static_paths).") .def("resolve_to_paths", [](const PredDAG& dag, std::int32_t src, std::int32_t dst, bool split_parallel_edges, py::object max_paths){ std::optional mp; if (!max_paths.is_none()) mp = py::cast(max_paths); @@ -478,12 +495,11 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { .def("get_flow_edges", [](const FlowGraph& fg, const FlowIndex& idx){ auto v = fg.get_flow_edges(idx); py::list out; for (auto const& pr : v) { out.append(py::make_tuple(pr.first, pr.second)); } return out; }) .def("get_flow_path", [](const FlowGraph& fg, const FlowIndex& idx){ auto v = fg.get_flow_path(idx); py::list out; for (auto e : v) out.append(e); return out; }); - py::class_(m, "Flow") - .def_property_readonly("index", [](const FlowRecord& f){ return f.index; }) - .def_readonly("src", &FlowRecord::src) - .def_readonly("dst", &FlowRecord::dst) - .def_readonly("cost", &FlowRecord::cost) - .def_readonly("placed_flow", &FlowRecord::placed_flow); + // NOTE: FlowRecord is deliberately not bound. No binding constructs or returns one + // (FlowPolicy.flows yields plain tuples), so the class was unreachable, and the + // Python name "Flow" collided with the C++ alias `using Flow = double`. If per-flow + // records become part of the public API, bind it under an unambiguous name such as + // "FlowRecord" and have FlowPolicy.flows return it instead of tuples. py::enum_(m, "PathAlg") .value("SPF", PathAlg::SPF); @@ -562,6 +578,9 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { py::class_(m, "FlowPolicy") .def("__init__", [](FlowPolicy& self, py::object algs_obj, const PyGraph& pg, FlowPolicyConfig cfg, py::object node_mask, py::object edge_mask){ + if (!py::isinstance(algs_obj)) { + throw py::type_error("FlowPolicy: algorithms must be an Algorithms instance"); + } std::shared_ptr algs = py::cast>(algs_obj); auto node_bs = to_bool_span_from_numpy(node_mask, static_cast(pg.num_nodes), "node_mask"); @@ -585,6 +604,17 @@ PYBIND11_MODULE(_netgraph_core, m, py::mod_gil_not_used()) { .def("rebalance_demand", [](FlowPolicy& p, FlowGraph& fg, std::int32_t src, std::int32_t dst, FlowClass flowClass, double target){ py::gil_scoped_release rel; auto pr = p.rebalance_demand(fg, src, dst, flowClass, target); py::gil_scoped_acquire acq; return py::make_tuple(pr.first, pr.second); }, py::arg("flow_graph"), py::arg("src"), py::arg("dst"), py::arg("flowClass"), py::arg("target")) .def("remove_demand", [](FlowPolicy& p, FlowGraph& fg){ py::gil_scoped_release rel; p.remove_demand(fg); py::gil_scoped_acquire acq; }) + .def("set_static_paths", [](FlowPolicy& p, std::int32_t src, std::int32_t dst, py::sequence paths){ + std::vector bundles; + bundles.reserve(py::len(paths)); + for (auto item : paths) bundles.push_back(py::cast(item)); + py::gil_scoped_release rel; + p.set_static_paths(src, dst, std::move(bundles)); + py::gil_scoped_acquire acq; + }, py::arg("src"), py::arg("dst"), py::arg("paths"), + "Pin this policy's demand to explicit path bundles (one flow per usable " + "bundle, bound in supply order). See FlowPolicy docs for validation, mask " + "(failure) semantics, and which config knobs become inert.") .def_property_readonly("flows", [](const FlowPolicy& p){ py::dict out; for (auto const& kv : p.flows()) { const auto& idx = kv.first; const auto& f = kv.second; out[py::make_tuple(idx.src, idx.dst, idx.flowClass, idx.flowId)] = py::make_tuple(f.src, f.dst, f.cost, f.placed_flow); } return out; }); // Profiling functions (enabled via NGRAPH_CORE_PROFILE=1 environment variable) diff --git a/include/netgraph/core/algorithms.hpp b/include/netgraph/core/algorithms.hpp index e7d3f98..c5ca458 100644 --- a/include/netgraph/core/algorithms.hpp +++ b/include/netgraph/core/algorithms.hpp @@ -16,10 +16,6 @@ class Algorithms { return backend_->build_graph(g); } - [[nodiscard]] GraphHandle build_graph(std::shared_ptr g) const { - return backend_->build_graph(std::move(g)); - } - [[nodiscard]] std::pair, PredDAG> spf(const GraphHandle& gh, NodeId src, const SpfOptions& opts) const { return backend_->spf(gh, src, opts); diff --git a/include/netgraph/core/backend.hpp b/include/netgraph/core/backend.hpp index 8ad21b6..1c2ba73 100644 --- a/include/netgraph/core/backend.hpp +++ b/include/netgraph/core/backend.hpp @@ -7,7 +7,6 @@ #pragma once #include -#include #include #include #include @@ -31,7 +30,9 @@ class Backend { // Prepares a backend-specific graph handle from an existing graph reference. // - // The CPU backend creates a non-owning shared_ptr with a no-op deleter. + // The CPU backend creates a non-owning shared_ptr with a no-op deleter, so the + // caller must keep `g` alive for as long as the handle is used. To hand the + // handle shared ownership instead, construct it directly: GraphHandle{my_sp}. // // Arguments: // g: The source graph to wrap. @@ -40,16 +41,6 @@ class Backend { // A GraphHandle containing the backend-specific graph representation. [[nodiscard]] virtual GraphHandle build_graph(const StrictMultiDiGraph& g) = 0; - // Prepares a backend-specific graph handle that takes shared ownership of the - // provided graph instance. - // - // Arguments: - // g: The source graph as a shared_ptr. - // - // Returns: - // A GraphHandle that shares ownership of the graph. - [[nodiscard]] virtual GraphHandle build_graph(std::shared_ptr g) = 0; - // Computes shortest paths from a source node. // // Arguments: @@ -121,8 +112,9 @@ class Backend { // opts: Configuration options. // // Returns: - // A vector of pairs (EdgeId, Flow gain), indicating how much flow would - // increase if the edge's capacity were relaxed. + // A vector of pairs (EdgeId, FlowDelta) for edges whose removal reduces + // total flow, where FlowDelta is how much flow is LOST without that edge. + // This is a criticality measure, not the gain from relaxing capacity. [[nodiscard]] virtual std::vector> sensitivity_analysis( const GraphHandle& gh, NodeId src, NodeId dst, const MaxFlowOptions& opts) = 0; }; diff --git a/include/netgraph/core/flow_policy.hpp b/include/netgraph/core/flow_policy.hpp index 8e9c4e4..32e1ea2 100644 --- a/include/netgraph/core/flow_policy.hpp +++ b/include/netgraph/core/flow_policy.hpp @@ -24,7 +24,6 @@ struct ExecutionContext { std::shared_ptr algorithms; GraphHandle graph; - // Constructor with validation ExecutionContext(std::shared_ptr algs, const GraphHandle& gh) noexcept : algorithms(std::move(algs)), graph(gh) {} }; @@ -97,7 +96,21 @@ class FlowPolicy { [[nodiscard]] int flow_count() const noexcept { return static_cast(flows_.size()); } [[nodiscard]] double placed_demand() const noexcept; - // Core operations + // Core operations. + // + // A FlowPolicy manages the flows of a SINGLE demand. Once it holds flows, calling + // place_demand() with a different (src, dst) throws std::invalid_argument, because + // the round-robin loop routes using the src/dst stored on the existing flow records. + // Use one policy per demand, or call remove_demand() first to retarget it. + // + // `fg` must wrap the same StrictMultiDiGraph as the policy's own graph handle; + // placing onto a FlowGraph built from a different graph is rejected. + // + // Returns (placed, remaining). NOTE for EqualBalanced: when the equalizing + // rebalance runs, `placed` is the policy's TOTAL placed demand (cumulative + // across calls, since rebalancing re-places previously placed volume too) and + // `placed + remaining` can therefore exceed this call's `volume`. Use + // placed_demand() deltas for strict per-call accounting. [[nodiscard]] std::pair place_demand(FlowGraph& fg, NodeId src, NodeId dst, FlowClass flowClass, @@ -105,6 +118,8 @@ class FlowPolicy { std::optional target_per_flow = std::nullopt, std::optional min_flow = std::nullopt); + // Re-places the currently placed volume so each flow carries ~target_per_flow. + // (src, dst) must match the demand this policy already manages. [[nodiscard]] std::pair rebalance_demand(FlowGraph& fg, NodeId src, NodeId dst, FlowClass flowClass, @@ -114,12 +129,49 @@ class FlowPolicy { [[nodiscard]] const std::unordered_map& flows() const noexcept { return flows_; } -// Configure static paths for flow creation. Each entry is (src, dst, dag, cost). -// max_flow_count must equal the number of static paths if set. - void set_static_paths(std::vector> paths); + // Pin this policy's demand to explicit path bundles (MPLS-style routing). + // + // One flow is created per USABLE bundle at the next placement, in supply order, + // each permanently bound to its own bundle. A single-path bundle (e.g. from + // make_path_dag / PredDAG.from_edges or ksp) models a strict-ERO LSP: any failed + // hop takes it down. A multi-walk bundle (e.g. an SPF DAG) models a pinned DAG / + // SR-TE-style policy that renormalizes over its surviving walks. + // + // Each bundle is validated against the policy's graph (shape, id ranges, + // edge-endpoint consistency, acyclicity, and an src->dst walk), then pruned + // against the policy's node/edge masks. A bundle with no surviving src->dst walk + // is DOWN: it creates no flow and carries nothing (a pinned path does not + // reroute around failures). Each flow's cost is the min-cost src->dst walk of + // its PRUNED bundle, computed from the graph's edge costs. + // + // With static paths configured the policy neither creates additional flows nor + // reoptimizes: max_path_cost, max_path_cost_factor, min_flow_count and + // reoptimize_flows_on_each_placement are inert. EqualBalanced spreads over the + // usable (up) bundles only. flow_count() reports the usable count U; the + // supplied count N is the caller's, so down LSPs = N - flow_count(). + // + // Throws std::invalid_argument if bundles is empty, the policy already holds + // flows (remove_demand() first), shortest_path=true is configured (single- + // augmentation IP semantics contradict pinned multi-LSP placement), src/dst is + // invalid or src == dst, a user-supplied max_flow_count differs from + // bundles.size(), or any bundle is malformed. Calling again before placement + // replaces the previous pinning. + void set_static_paths(NodeId src, NodeId dst, std::vector bundles); private: // Helpers + // Throws std::invalid_argument if fg wraps a different graph than this policy, or if + // (src, dst) differs from the demand already managed here. `what` names the caller. + void check_demand_target(const FlowGraph& fg, NodeId src, NodeId dst, const char* what) const; + // Placement core: target computation, flow creation, round-robin placement, and + // the optional post-placement reoptimization. The public place_demand() wraps it + // with the iterative EqualBalanced rebalance rounds. + [[nodiscard]] std::pair place_demand_body(FlowGraph& fg, + NodeId src, NodeId dst, + FlowClass flowClass, + double volume, + std::optional target_per_flow, + std::optional min_flow); [[nodiscard]] std::optional> get_path_bundle(const FlowGraph& fg, NodeId src, NodeId dst, std::optional min_flow); @@ -157,8 +209,17 @@ class FlowPolicy { Cost best_path_cost_ { std::numeric_limits::max() }; FlowId next_flow_id_ { 0 }; - // Static paths (optional) - std::vector> static_paths_; + // Static paths (optional): usable (mask-pruned) bundles, one flow per bundle. + struct StaticBundle { + PredDAG dag; // pruned to mask-surviving entries + Cost cost {0}; // min-cost src->dst walk within the pruned bundle + }; + NodeId static_src_ {-1}; + NodeId static_dst_ {-1}; + int static_supplied_count_ {0}; // N as supplied; usable U = static_bundles_.size() + std::vector static_bundles_; + + [[nodiscard]] bool has_static_paths() const noexcept { return !static_bundles_.empty() || static_supplied_count_ > 0; } }; } // namespace netgraph::core diff --git a/include/netgraph/core/flow_state.hpp b/include/netgraph/core/flow_state.hpp index afd510c..bdc327e 100644 --- a/include/netgraph/core/flow_state.hpp +++ b/include/netgraph/core/flow_state.hpp @@ -7,7 +7,6 @@ #pragma once #include -#include #include #include #include @@ -90,6 +89,9 @@ class FlowState { // Apply or revert a set of edge flow allocations directly. // When add==true, treats each (eid, flow) as additional placed flow on the edge. // When add==false, removes placed flow (reverts allocations), clamping to [0, capacity]. + // NOTE: residual is recomputed as capacity - edge_flow, so for a FlowState built with + // a custom residual_init this discards the initial offset. Entries with flow <= 0 and + // out-of-range edge ids are ignored. void apply_deltas(std::span> deltas, bool add) noexcept; private: diff --git a/include/netgraph/core/max_flow.hpp b/include/netgraph/core/max_flow.hpp index 08e8391..347b30d 100644 --- a/include/netgraph/core/max_flow.hpp +++ b/include/netgraph/core/max_flow.hpp @@ -1,7 +1,6 @@ /* Max-flow utility APIs with summaries and batch evaluation. */ #pragma once -#include #include #include #include @@ -16,8 +15,14 @@ struct MinCut { std::vector edges; }; struct FlowSummary { Flow total_flow {0.0}; MinCut min_cut {}; - // Parallel arrays: costs[i] has placed flow flows[i] at that total path cost. - // Costs are ascending and unique; flows are raw volumes (not normalized). + // Parallel arrays: costs[i] carries placed flow flows[i]. Costs are ascending and + // unique; flows are raw volumes (not normalized). + // + // Entries from the SPF tier loop are the total cost of the shortest-path DAG used. + // Entries produced by the residual completion phase are MARGINAL costs: the cost of + // the augmenting path's forward edges minus the cost of the flow it cancelled. Such + // an entry need not correspond to any traversable s-t path, so treat costs[] as a + // cost-weighted breakdown of the total flow rather than a list of path costs. std::vector costs; std::vector flows; std::vector edge_flows; // filled if requested @@ -26,6 +31,17 @@ struct FlowSummary { std::vector reachable_nodes; // length == g.num_nodes(); 0/1 flags }; +// Computes maximum flow from src to dst. +// +// The SPF tier loop augments along forward shortest-path DAGs only. For +// FlowPlacement::Proportional with require_capacity=true and shortest_path=false a +// residual completion phase then augments with reverse arcs, so the result is a true +// maximum flow and summary.min_cut satisfies max-flow/min-cut duality. +// +// The other configurations are placement models rather than max-flow computations and +// may report less than the maximum: EqualBalanced models single-pass ECMP admission, +// require_capacity=false models fixed cost-only IP routing, and shortest_path=true +// restricts flow to the first cost tier. [[nodiscard]] std::pair calc_max_flow(const StrictMultiDiGraph& g, NodeId src, NodeId dst, FlowPlacement placement, bool shortest_path, diff --git a/include/netgraph/core/profiling.hpp b/include/netgraph/core/profiling.hpp index 0561d3d..40eb9ea 100644 --- a/include/netgraph/core/profiling.hpp +++ b/include/netgraph/core/profiling.hpp @@ -2,8 +2,7 @@ Enable via environment variable: NGRAPH_CORE_PROFILE=1 -When disabled (default), overhead is minimal: ~2-4 CPU cycles per scope -due to a single static bool check with branch prediction. +When disabled (default), each instrumented scope costs one cached-bool check. Usage: #include "netgraph/core/profiling.hpp" @@ -22,7 +21,6 @@ From Python: */ #pragma once -#include #include #include #include @@ -113,8 +111,8 @@ class ScopedTimer { #define NGRAPH_CONCAT_IMPL(a, b) a##b #define NGRAPH_CONCAT(a, b) NGRAPH_CONCAT_IMPL(a, b) -// Main profiling macro. Expands to a ScopedTimer only if profiling is enabled. -// When disabled, the check is a single static bool read (~1-2 cycles). +// Main profiling macro. Expands to a ScopedTimer only if profiling is enabled; +// when disabled the timer is constructed with a null name and does no work. #define NGRAPH_PROFILE_SCOPE(name) \ ::netgraph::core::ScopedTimer NGRAPH_CONCAT(_ngraph_timer_, __LINE__)( \ ::netgraph::core::profiling_enabled() ? (name) : nullptr) diff --git a/include/netgraph/core/shortest_paths.hpp b/include/netgraph/core/shortest_paths.hpp index 47924f9..6c91d3d 100644 --- a/include/netgraph/core/shortest_paths.hpp +++ b/include/netgraph/core/shortest_paths.hpp @@ -11,9 +11,16 @@ namespace netgraph::core { -// PredDAG (Predecessor Directed Acyclic Graph): compact representation of all equal-cost +// PredDAG (Predecessor Directed Acyclic Graph): compact representation of the equal-cost // shortest paths from a source node. Stored in CSR format for efficiency. // +// The DAG is always acyclic. With strictly positive edge costs it captures *every* +// equal-cost shortest path. Zero-cost edges are the one exception: a predecessor is +// recorded only while the child is still unsettled, so among nodes that are mutually +// reachable at equal distance the settle order decides which alternatives are kept. +// Without that rule a zero-cost pair u<->v would record each node as the other's +// parent, making the structure cyclic and path enumeration non-terminating. +// // For each node v, predecessors are stored in parents[parent_offsets[v]:parent_offsets[v+1]] // with corresponding EdgeIds in via_edges[parent_offsets[v]:parent_offsets[v+1]]. // Multiple parallel edges are represented by multiple entries with the same parent. @@ -52,10 +59,34 @@ shortest_paths(const StrictMultiDiGraph& g, NodeId src, std::span node_mask = {}, std::span edge_mask = {}); +// Build the SPF-compatible (distances, PredDAG) pair for one concrete path given +// as node/edge sequences (nodes[i] -> nodes[i+1] via edges[i]; so nodes.size() == +// edges.size() + 1, or a single node with no edges). Distances are cumulative edge +// costs along the path, INT64_MAX elsewhere; the DAG stores one parent per visited +// node. Inputs are trusted (no validation) -- used by KSP's result conversion. +[[nodiscard]] std::pair, PredDAG> +path_to_pred_dag(const StrictMultiDiGraph& g, + std::span nodes, + std::span edges); + +// Build a single-path PredDAG from a contiguous edge sequence (the missing +// constructor for operator-defined explicit paths; PredDAGs from SPF/KSP work +// directly wherever a PredDAG is accepted). +// +// Throws std::invalid_argument if edges is empty, any id is out of range, the +// sequence is not contiguous (dst(e_i) != src(e_{i+1})), or the walk repeats a +// node (a repeated node cannot be represented as one-parent-per-node, and a +// pinned path is a simple path by definition). +[[nodiscard]] PredDAG make_path_dag(const StrictMultiDiGraph& g, + std::span edges); + // Enumerate concrete paths represented by a PredDAG from src to dst. // Each path is returned as a sequence of (node_id, (edge_ids...)) pairs ending with (dst, ()). // When split_parallel_edges=false, parallel edges per hop are grouped in the tuple. -// When true, one edge per hop is selected to produce concrete paths; enumeration may be capped with max_paths. +// When true, the cartesian product over parallel edges is enumerated, yielding one +// concrete single-edge path per combination; cap it with max_paths. +// Enumeration is exponential in the DAG's branching, so pass max_paths on wide ECMP DAGs. +// NOTE: this function does not release the Python GIL when called through the bindings. [[nodiscard]] std::vector>>> resolve_to_paths(const PredDAG& dag, NodeId src, NodeId dst, bool split_parallel_edges = false, diff --git a/include/netgraph/core/strict_multidigraph.hpp b/include/netgraph/core/strict_multidigraph.hpp index deb2890..5f14498 100644 --- a/include/netgraph/core/strict_multidigraph.hpp +++ b/include/netgraph/core/strict_multidigraph.hpp @@ -18,6 +18,18 @@ namespace netgraph::core { class StrictMultiDiGraph { public: + // Builds a graph from parallel edge arrays, which must all have the same length. + // Edges are reordered by (cost, src, dst); ext_edge_ids is permuted alongside them. + // Self-loops and duplicate (src, dst) pairs are permitted (this is a multigraph). + // + // Throws std::invalid_argument or std::out_of_range if: + // - the arrays differ in length, or ext_edge_ids is neither empty nor that length + // - num_nodes < 0, or any src/dst falls outside [0, num_nodes) + // - any capacity < 0, or any cost < 0 + // - the number of edges exceeds INT32_MAX + // - the TOTAL of all edge costs reaches 2^62. SPF accumulates path costs as int64 + // and uses INT64_MAX as the unreachable sentinel, so a larger total could wrap + // negative and silently corrupt results. [[nodiscard]] static StrictMultiDiGraph from_arrays( std::int32_t num_nodes, std::span src, diff --git a/pyproject.toml b/pyproject.toml index a0aedec..5ab88d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,10 +1,10 @@ [build-system] -requires = ["scikit-build-core>=0.10", "pybind11>=3", "numpy>=1.22"] +requires = ["scikit-build-core>=0.10", "pybind11>=3", "numpy>=1.23.2"] build-backend = "scikit_build_core.build" [project] name = "netgraph-core" -version = "0.7.2" +version = "0.8.0" description = "C++ implementation of graph algorithms for network flow analysis and traffic engineering with Python bindings" readme = "README.md" requires-python = ">=3.11" @@ -26,7 +26,7 @@ classifiers = [ "Programming Language :: Python :: Implementation :: CPython", "Operating System :: OS Independent", ] -dependencies = ["numpy>=1.22"] +dependencies = ["numpy>=1.23.2"] [project.optional-dependencies] dev = [ diff --git a/python/netgraph_core/__init__.py b/python/netgraph_core/__init__.py index f8180ff..3bb8a27 100644 --- a/python/netgraph_core/__init__.py +++ b/python/netgraph_core/__init__.py @@ -8,6 +8,7 @@ # pyright: reportMissingImports=false from importlib.metadata import version +from typing import TYPE_CHECKING from _netgraph_core import ( Algorithms, @@ -34,21 +35,32 @@ __version__ = version("netgraph-core") # Provide richer type information for editors/type-checkers without affecting runtime. -try: # pragma: no cover - typing-only import - from typing import TYPE_CHECKING as _TYPE_CHECKING - - if _TYPE_CHECKING: # noqa: SIM108 - from ._docs import ( # noqa: I001 - EdgeSelection as EdgeSelection, - EdgeTieBreak as EdgeTieBreak, - FlowPlacement as FlowPlacement, - FlowSummary as FlowSummary, - MinCut as MinCut, - PredDAG as PredDAG, - ) -except ImportError: - # Safe fallback if _docs.py changes; runtime bindings above remain authoritative. - pass +# Every name here must also appear in the runtime import above; the aliases only +# change what a type checker sees. Keep this list in sync with _docs.py, and keep +# _docs.py in sync with the bindings -- a wrong stub is worse than a missing one, +# because it turns "unchecked" into "confidently wrong". +if TYPE_CHECKING: # pragma: no cover - typing-only + from ._docs import ( # noqa: I001 + Algorithms as Algorithms, + Backend as Backend, + EdgeSelection as EdgeSelection, + EdgeTieBreak as EdgeTieBreak, + FlowGraph as FlowGraph, + FlowIndex as FlowIndex, + FlowPlacement as FlowPlacement, + FlowPolicy as FlowPolicy, + FlowPolicyConfig as FlowPolicyConfig, + FlowState as FlowState, + FlowSummary as FlowSummary, + Graph as Graph, + MinCut as MinCut, + PathAlg as PathAlg, + PredDAG as PredDAG, + StrictMultiDiGraph as StrictMultiDiGraph, + profiling_dump as profiling_dump, + profiling_enabled as profiling_enabled, + profiling_reset as profiling_reset, + ) __all__ = [ "__version__", diff --git a/python/netgraph_core/_docs.py b/python/netgraph_core/_docs.py index 06dc98d..c873f0e 100644 --- a/python/netgraph_core/_docs.py +++ b/python/netgraph_core/_docs.py @@ -9,9 +9,7 @@ from __future__ import annotations -from dataclasses import dataclass -from enum import Enum -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, ClassVar, Optional, Sequence if TYPE_CHECKING: # only for typing; runtime comes from extension import numpy as np # type: ignore[reportMissingImports] @@ -19,19 +17,38 @@ # to prevent circular imports during type checking. -class EdgeTieBreak(Enum): - DETERMINISTIC = 1 - PREFER_HIGHER_RESIDUAL = 2 +class EdgeTieBreak: + """Tie-break rule among equal-cost parallel edges (pybind11 enum).""" + + DETERMINISTIC: ClassVar[EdgeTieBreak] + PREFER_HIGHER_RESIDUAL: ClassVar[EdgeTieBreak] + __members__: ClassVar[dict[str, EdgeTieBreak]] + + def __init__(self, value: int) -> None: ... + + @property + def name(self) -> str: ... + @property + def value(self) -> int: ... -@dataclass class EdgeSelection: - multi_edge: bool = True - require_capacity: bool = False - tie_break: EdgeTieBreak = EdgeTieBreak.DETERMINISTIC + """Edge selection policy. The constructor is keyword-only.""" + + multi_edge: bool + require_capacity: bool + tie_break: EdgeTieBreak + + def __init__( + self, + *, + multi_edge: bool = True, + require_capacity: bool = False, + tie_break: EdgeTieBreak = ..., + ) -> None: ... -class FlowPlacement(Enum): +class FlowPlacement: """How to place flow across equal-cost predecessors during augmentation. PROPORTIONAL (WCMP-like): Distributes flow proportionally to available capacity. @@ -44,8 +61,16 @@ class FlowPlacement(Enum): ECMP = Equal-Cost Multi-Path; WCMP = Weighted-Cost Multi-Path. """ - PROPORTIONAL = 1 - EQUAL_BALANCED = 2 + PROPORTIONAL: ClassVar[FlowPlacement] + EQUAL_BALANCED: ClassVar[FlowPlacement] + __members__: ClassVar[dict[str, FlowPlacement]] + + def __init__(self, value: int) -> None: ... + + @property + def name(self) -> str: ... + @property + def value(self) -> int: ... class PredDAG: @@ -58,6 +83,22 @@ class PredDAG: parents: np.ndarray via_edges: np.ndarray + @staticmethod + def from_edges(graph: "StrictMultiDiGraph", edges: "Sequence[int]") -> "PredDAG": + """Build a single-path PredDAG from a contiguous edge-id sequence. + + The missing constructor for operator-defined explicit paths; PredDAGs + returned by Algorithms.spf/ksp work directly wherever a PredDAG is + accepted (FlowState.place_on_dag, FlowGraph.place, + FlowPolicy.set_static_paths). + + Raises: + ValueError: If edges is empty, an id is out of range, consecutive + edges do not connect, or the walk revisits a node (a pinned + path must be a simple path). + """ + ... + def resolve_to_paths( self, src: int, @@ -68,8 +109,16 @@ def resolve_to_paths( ) -> list[tuple[tuple[int, tuple[int, ...]], ...]]: ... -class PathAlg(Enum): - SPF = 1 +class PathAlg: + SPF: ClassVar[PathAlg] + __members__: ClassVar[dict[str, PathAlg]] + + def __init__(self, value: int) -> None: ... + + @property + def name(self) -> str: ... + @property + def value(self) -> int: ... class Backend: @@ -82,9 +131,15 @@ def cpu() -> "Backend": class Graph: - """Opaque graph handle provided by the runtime extension (typing stub only).""" + """Opaque backend graph handle produced by Algorithms.build_graph. - ... + Keeps the underlying StrictMultiDiGraph alive for as long as it is used. + """ + + @property + def num_nodes(self) -> int: ... + @property + def num_edges(self) -> int: ... class FlowGraph: @@ -118,7 +173,7 @@ def place( dst: int, dag: "PredDAG", amount: float, - flow_placement: FlowPlacement = FlowPlacement.PROPORTIONAL, + flow_placement: FlowPlacement = ..., ) -> float: ... def remove(self, index: "FlowIndex") -> None: ... @@ -187,7 +242,7 @@ def place_on_dag( dst: int, dag: "PredDAG", requested_flow: float = float("inf"), - flow_placement: FlowPlacement = FlowPlacement.PROPORTIONAL, + flow_placement: FlowPlacement = ..., ) -> float: """Place flow along a predecessor DAG. @@ -205,7 +260,7 @@ def place_max_flow( self, src: int, dst: int, - flow_placement: FlowPlacement = FlowPlacement.PROPORTIONAL, + flow_placement: FlowPlacement = ..., shortest_path: bool = False, require_capacity: bool = True, *, @@ -256,16 +311,107 @@ def compute_min_cut( class StrictMultiDiGraph: - """Opaque graph structure provided by the runtime extension (typing stub only).""" + """Immutable directed multigraph with CSR and reverse-CSR adjacency. - ... + Edges are reordered by (cost, src, dst) at construction, so EdgeId values are + positions in that canonical order rather than the caller's input order. Use + `ext_edge_ids` to map results back to caller-side identifiers. + + Every `*_view()` method returns a fresh, writable COPY of the internal buffer, + so mutating the result cannot violate the graph's immutability. (This differs + from `FlowState`/`FlowGraph` views, which are live read-only views.) + """ + + @staticmethod + def from_arrays( + num_nodes: int, + src: "np.ndarray", + dst: "np.ndarray", + capacity: "np.ndarray", + cost: "np.ndarray", + ext_edge_ids: "np.ndarray", + ) -> "StrictMultiDiGraph": + """Build a graph from parallel edge arrays (all must be 1-D, C-contiguous). + + Args: + num_nodes: Node count; every src/dst must lie in [0, num_nodes). + src: int32[E] source node ids. + dst: int32[E] destination node ids. + capacity: float64[E] capacities, each >= 0. + cost: int64[E] costs, each >= 0. Their TOTAL must stay below 2**62, + since SPF accumulates path costs as int64. + ext_edge_ids: int64[E] caller-side ids, permuted alongside the edges. + + Raises: + TypeError: On a wrong dtype, a non-contiguous array, or a wrong shape. + ValueError: On a negative capacity/cost, a node id out of range, a + length mismatch, or a total cost at or above 2**62. + """ + ... + + def num_nodes(self) -> int: ... + def num_edges(self) -> int: ... + def capacity_view(self) -> "np.ndarray": + """Copy of per-edge capacities, float64[E].""" + ... + + def cost_view(self) -> "np.ndarray": + """Copy of per-edge costs, int64[E].""" + ... + + def edge_src_view(self) -> "np.ndarray": + """Copy of per-edge source node ids, int32[E].""" + ... + + def edge_dst_view(self) -> "np.ndarray": + """Copy of per-edge destination node ids, int32[E].""" + ... + + def ext_edge_ids_view(self) -> "np.ndarray": + """Copy of caller-supplied external edge ids, int64[E].""" + ... + + def row_offsets_view(self) -> "np.ndarray": + """CSR row offsets over outgoing edges, int32[num_nodes + 1].""" + ... + + def col_indices_view(self) -> "np.ndarray": + """CSR neighbour node ids for outgoing edges, int32[E].""" + ... + + def adj_edge_index_view(self) -> "np.ndarray": + """EdgeId for each CSR outgoing entry, int32[E].""" + ... + + def in_row_offsets_view(self) -> "np.ndarray": + """Reverse-CSR row offsets over incoming edges, int32[num_nodes + 1].""" + ... + + def in_col_indices_view(self) -> "np.ndarray": + """Reverse-CSR predecessor node ids, int32[E].""" + ... + + def in_adj_edge_index_view(self) -> "np.ndarray": + """EdgeId for each reverse-CSR entry, int32[E].""" + ... class FlowIndex: - src: int - dst: int - flowClass: int - flowId: int + """Identity of one flow: (src, dst, flowClass, flowId). + + The constructor takes exactly four POSITIONAL arguments (the binding + declares no argument names), and the attributes are read-only. + """ + + def __init__(self, src: int, dst: int, flowClass: int, flowId: int, /) -> None: ... + @property + def src(self) -> int: ... + @property + def dst(self) -> int: ... + @property + def flowClass(self) -> int: ... + @property + def flowId(self) -> int: ... class FlowPolicyConfig: @@ -298,13 +444,39 @@ class FlowPolicyConfig: diminishing_returns_window: int diminishing_returns_epsilon_frac: float + def __init__( + self, + *, + path_alg: PathAlg = ..., + flow_placement: FlowPlacement = ..., + selection: EdgeSelection = ..., + require_capacity: bool = True, + multipath: bool = True, + min_flow_count: int = 1, + max_flow_count: Optional[int] = None, + max_path_cost: Optional[int] = None, + max_path_cost_factor: Optional[float] = None, + shortest_path: bool = False, + reoptimize_flows_on_each_placement: bool = False, + max_no_progress_iterations: int = 100, + max_total_iterations: int = 10000, + diminishing_returns_enabled: bool = True, + diminishing_returns_window: int = 8, + diminishing_returns_epsilon_frac: float = 1e-3, + ) -> None: + """All parameters are keyword-only; the no-argument form is also valid.""" + ... + class FlowPolicy: """Flow policy for demand placement. - When static_paths is empty the policy may refresh the DAG per round using - residual-aware shortest paths. This progressively prunes saturated next-hops - (traffic-engineering style) and differs from one-shot ECMP admission. + Dynamic policies may refresh the DAG per round using residual-aware + shortest paths, progressively pruning saturated next-hops (traffic- + engineering style), which differs from one-shot ECMP admission. + + A policy can instead be pinned to explicit path bundles with + set_static_paths(); see that method for the pinned semantics. Args: algorithms: Algorithms instance (kept alive by FlowPolicy) @@ -338,7 +510,24 @@ def place_demand( volume: float, target_per_flow: Optional[float] = None, min_flow: Optional[float] = None, - ) -> tuple[float, float]: ... + ) -> tuple[float, float]: + """Place `volume` of demand; returns (placed, remaining). + + Raises: + ValueError: If this policy already manages a different (src, dst) + pair (call remove_demand() first, or use one policy per + demand), or if `flow_graph` wraps a different + StrictMultiDiGraph than the policy's own graph handle. + + EQUAL_BALANCED note: when the equalizing rebalance runs, `placed` is + the policy's TOTAL placed demand (cumulative across calls, because + rebalancing re-places previously placed volume too), so + `placed + remaining` can exceed this call's `volume`. Use + placed_demand() deltas for strict per-call accounting. EB placement is + also not incremental: each call's volume defines the per-flow target, + so a second, smaller call may place nothing. + """ + ... def rebalance_demand( self, @@ -349,27 +538,59 @@ def rebalance_demand( target: float, ) -> tuple[float, float]: ... - def remove_demand(self, flow_graph: "FlowGraph") -> None: ... + def remove_demand(self, flow_graph: "FlowGraph") -> None: + """Remove all of this policy's flows, releasing its (src, dst) binding.""" + ... - @property - def flows(self) -> dict[tuple[int, int, int, int], tuple[int, int, int, float]]: ... + def set_static_paths(self, src: int, dst: int, paths: "Sequence[PredDAG]") -> None: + """Pin this policy's demand to explicit path bundles (MPLS-style). + One flow is created per USABLE bundle at the next placement, in supply + order, each permanently bound to its own bundle. A single-path bundle + (from PredDAG.from_edges or ksp) models a strict-ERO LSP: any failed + hop takes it down. A multi-walk bundle (an SPF DAG) models a pinned + DAG / SR-TE-style policy that renormalizes over surviving walks. -@dataclass(frozen=True) -class Path: - nodes: np.ndarray - edges: np.ndarray - cost: float + Bundles are validated against the policy's graph, then pruned against + the policy's node/edge masks; a bundle with no surviving src->dst walk + is DOWN and creates no flow (pinned paths do not reroute around + failures). Each flow's cost is the min-cost src->dst walk of its + PRUNED bundle. EqualBalanced spreads over the usable (up) bundles + only; down LSPs = len(paths) - flow_count(). + + With static paths configured the policy neither creates additional + flows nor reoptimizes: max_path_cost, max_path_cost_factor, + min_flow_count and reoptimize_flows_on_each_placement are inert. + Placement is not incremental across place_demand calls for + EqualBalanced (each call's volume defines the per-flow target). + + Raises: + ValueError: If paths is empty; the policy already holds flows + (call remove_demand() first); shortest_path=True is + configured; src/dst is out of range or equal; a user-supplied + max_flow_count differs from len(paths); or a bundle is + malformed (wrong shape, out-of-range ids, an edge that does + not connect its parent to its node in this graph, a cycle, or + no src->dst walk). Calling again before placement replaces + the previous pinning. + """ + ... + + @property + def flows(self) -> dict[tuple[int, int, int, int], tuple[int, int, int, float]]: ... class MinCut: - edges: list[int] + edges: "np.ndarray" # int32[K], copy of the cut edge ids -@dataclass(frozen=True) class FlowSummary: total_flow: float min_cut: MinCut + # Cost-weighted breakdown of total_flow, ascending and unique. Entries from + # the SPF tier loop are shortest-path-DAG costs; entries from the residual + # completion phase are MARGINAL costs (forward edge costs minus the cost of + # the flow they cancel) and need not match any traversable src->dst path. costs: "np.ndarray" # int64[K] flows: "np.ndarray" # float64[K] edge_flows: "np.ndarray" @@ -401,18 +622,6 @@ def build_graph(self, graph: "StrictMultiDiGraph") -> "Graph": """ ... - def build_graph_from_arrays( - self, - num_nodes: int, - src: "np.ndarray", - dst: "np.ndarray", - capacity: "np.ndarray", - cost: "np.ndarray", - ext_edge_ids: "np.ndarray", - ) -> "Graph": - """Build graph directly from arrays (graph is owned by the handle).""" - ... - def spf( self, graph: "Graph", @@ -451,7 +660,9 @@ def spf( as no traversal can begin from an excluded source. Raises: - TypeError: If arrays have wrong dtype, ndim, or length. + TypeError: If arrays have wrong dtype, ndim, or length + (residual must have length num_edges; masks must match + num_nodes / num_edges). ValueError: If src/dst out of range. """ ... @@ -491,7 +702,7 @@ def max_flow( src: int, dst: int, *, - flow_placement: FlowPlacement = FlowPlacement.PROPORTIONAL, + flow_placement: FlowPlacement = ..., shortest_path: bool = False, require_capacity: bool = True, with_edge_flows: bool = False, @@ -540,7 +751,7 @@ def batch_max_flow( *, node_masks: Optional[list["np.ndarray"]] = None, edge_masks: Optional[list["np.ndarray"]] = None, - flow_placement: FlowPlacement = FlowPlacement.PROPORTIONAL, + flow_placement: FlowPlacement = ..., shortest_path: bool = False, require_capacity: bool = True, with_edge_flows: bool = False, @@ -566,7 +777,7 @@ def sensitivity_analysis( src: int, dst: int, *, - flow_placement: FlowPlacement = FlowPlacement.PROPORTIONAL, + flow_placement: FlowPlacement = ..., shortest_path: bool = False, require_capacity: bool = True, node_mask: Optional["np.ndarray"] = None, @@ -602,3 +813,18 @@ def sensitivity_analysis( whose removal would reduce total flow by flow_delta. """ ... + + +def profiling_enabled() -> bool: + """True if profiling is enabled (NGRAPH_CORE_PROFILE=1 in the environment).""" + ... + + +def profiling_dump() -> None: + """Print collected profiling statistics to stderr.""" + ... + + +def profiling_reset() -> None: + """Clear all collected profiling statistics.""" + ... diff --git a/src/cpu_backend.cpp b/src/cpu_backend.cpp index a37a131..2f64537 100644 --- a/src/cpu_backend.cpp +++ b/src/cpu_backend.cpp @@ -18,16 +18,14 @@ class CpuBackend final : public Backend { return GraphHandle{ std::shared_ptr(&g, [](const StrictMultiDiGraph*){}) }; } - GraphHandle build_graph(std::shared_ptr g) override { - return GraphHandle{ std::move(g) }; - } - std::pair, PredDAG> spf( const GraphHandle& gh, NodeId src, const SpfOptions& opts) override { const StrictMultiDiGraph& g = *gh.graph; // Validate mask lengths strictly; mismatches are user errors. - // NOTE: Keep this as the single public boundary check; deeper layers - // (shortest_paths) should rely on this to avoid redundant validation. + // NOTE: this check is deliberately duplicated with the one inside + // shortest_paths(): the backend layer produces a caller-facing message, and the + // algorithm layer must stand on its own because it is also part of the public + // C++ API and callable without going through a Backend. if (!opts.node_mask.empty() && opts.node_mask.size() != static_cast(g.num_nodes())) { throw std::invalid_argument("CpuBackend::spf: node_mask length mismatch"); } diff --git a/src/flow_policy.cpp b/src/flow_policy.cpp index 0201bd1..3e1a773 100644 --- a/src/flow_policy.cpp +++ b/src/flow_policy.cpp @@ -22,10 +22,34 @@ #include #include #include -#include namespace netgraph::core { +/* Reject uses that would silently produce a wrong answer: + - a FlowGraph wrapping a different graph than the policy routes on (SPF would run + on one topology while flow is placed on another); + - a (src, dst) pair different from the demand this policy already manages (the + round-robin loop routes using src/dst from the existing flow records). */ +void FlowPolicy::check_demand_target(const FlowGraph& fg, NodeId src, NodeId dst, + const char* what) const { + const auto* policy_graph = ctx_.graph.graph.get(); + if (policy_graph != nullptr && policy_graph != &fg.graph()) { + throw std::invalid_argument( + std::string("FlowPolicy::") + what + + ": the FlowGraph wraps a different StrictMultiDiGraph than this policy; " + "paths would be selected on one topology and placed on another"); + } + if (!flows_.empty()) { + const auto& existing = flows_.begin()->second; + if (existing.src != src || existing.dst != dst) { + throw std::invalid_argument( + std::string("FlowPolicy::") + what + + ": this policy already manages a demand for a different (src, dst) pair; " + "use a separate FlowPolicy per demand or call remove_demand() first"); + } + } +} + double FlowPolicy::placed_demand() const noexcept { double s = 0.0; for (auto const& kv : flows_) s += kv.second.placed_flow; @@ -37,16 +61,10 @@ double FlowPolicy::placed_demand() const noexcept { std::optional> FlowPolicy::get_path_bundle(const FlowGraph& fg, NodeId src, NodeId dst, std::optional min_flow) { - // Static path handling: if static paths are configured, use them exclusively. - if (!static_paths_.empty()) { - // Search for a static path matching this (src, dst) pair. - for (auto const& t : static_paths_) { - if (std::get<0>(t) == src && std::get<1>(t) == dst) { - return std::make_optional(std::make_pair(std::get<2>(t), std::get<3>(t))); - } - } - return std::nullopt; // No static path for this pair - } + // With static paths, flows are created directly from the pinned bundles in + // place_demand_body and never reoptimized, so no dynamic bundle exists to hand + // out. Callers reaching here with static paths configured get nothing. + if (has_static_paths()) return std::nullopt; if (path_alg_ != PathAlg::SPF) return std::nullopt; // Use configured selection for per-adjacency edge behavior (multi-edge, tie-breaking). @@ -127,9 +145,19 @@ std::optional> FlowPolicy::get_path_bundle(const FlowGr // max_path_cost: absolute upper bound on path cost. // max_path_cost_factor: relative multiplier on best path cost (e.g. 1.5 = allow 50% longer). if (max_path_cost_.has_value() || max_path_cost_factor_.has_value()) { - double maxf = max_path_cost_factor_.value_or(1.0); - Cost absmax = max_path_cost_.value_or(std::numeric_limits::max()); - if (dst_cost > std::min(absmax, static_cast(static_cast(best_path_cost_) * maxf))) return std::nullopt; + const Cost absmax = max_path_cost_.value_or(std::numeric_limits::max()); + // Apply the relative bound only when a best cost exists: multiplying the + // INT64_MAX "no best yet" sentinel by the factor and casting back is + // undefined behavior (the product exceeds the int64 range). + Cost factor_bound = std::numeric_limits::max(); + if (max_path_cost_factor_.has_value() && + best_path_cost_ != std::numeric_limits::max()) { + const double prod = static_cast(best_path_cost_) * *max_path_cost_factor_; + if (prod < static_cast(std::numeric_limits::max())) { + factor_bound = static_cast(prod); + } + } + if (dst_cost > std::min(absmax, factor_bound)) return std::nullopt; } // Ensure there is at least one predecessor for dst if (static_cast(dst) >= dag.parent_offsets.size()-1) return std::nullopt; @@ -165,6 +193,11 @@ FlowRecord* FlowPolicy::create_flow(FlowGraph& fg, NodeId src, NodeId dst, FlowC Reoptimization is useful when a flow's current path becomes suboptimal due to network changes or when seeking additional capacity. */ FlowRecord* FlowPolicy::reoptimize_flow(FlowGraph& fg, const FlowIndex& idx, double headroom) { + // Pinned means pinned: a static-path flow is never rerouted. (Deliberate + // divergence from the original Python port, where reoptimization could + // silently move a pinned flow onto an SPF path.) Must precede any + // remove/re-place churn below. + if (has_static_paths()) return nullptr; auto it = flows_.find(idx); if (it == flows_.end()) return nullptr; FlowRecord& cur = it->second; @@ -202,15 +235,83 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, std::optional min_flow) { NGRAPH_PROFILE_SCOPE("place_demand"); + check_demand_target(fg, src, dst, "place_demand"); + + auto [total_placed, remaining] = place_demand_body(fg, src, dst, flowClass, volume, + target_per_flow, min_flow); + + // For EQUAL_BALANCED placement, rebalance flows to maintain equal volumes. + // + // Iterative on purpose: the previous implementation recursed + // place_demand -> rebalance_demand -> place_demand until balanced, and with many + // pinned bundles of heterogeneous capacity the depth grows like + // U * ln(imbalance / kMinFlow) -- a stack-overflow risk on worker threads. Each + // round below performs exactly what one recursion level performed (re-place the + // currently placed volume at the equal-share target), and the recursion's return + // value telescoped to (placed_demand(), pre-rebalance leftover + volume lost in + // rebalancing), which is reproduced after the loop. + if (flow_placement_ == FlowPlacement::EqualBalanced && !flows_.empty()) { + // Restore the reoptimize flag even if a round throws (bad_alloc is the only + // realistic thrower here); otherwise the policy would stay permanently + // non-reoptimizing. + struct ReoptRestore { + bool& flag; + bool prev; + ~ReoptRestore() { flag = prev; } + } reopt_restore{reoptimize_flows_on_each_placement_, reoptimize_flows_on_each_placement_}; + reoptimize_flows_on_each_placement_ = false; + const double pre_rounds_placed = placed_demand(); + bool rebalanced = false; + // Backstop only; each round strictly reduces the imbalance in practice. + constexpr int kMaxRebalanceRounds = 65536; + for (int round = 0; round < kMaxRebalanceRounds && !flows_.empty(); ++round) { + const double target_eq = placed_demand() / static_cast(flows_.size()); + bool unbalanced = false; + for (auto const& kv : flows_) { + if (std::abs(target_eq - kv.second.placed_flow) >= kMinFlow) { unbalanced = true; break; } + } + if (!unbalanced) break; + rebalanced = true; + const double vol = placed_demand(); + remove_demand(fg); + (void)place_demand_body(fg, src, dst, flowClass, vol, target_eq, std::nullopt); + } + if (rebalanced) { + const double final_placed = placed_demand(); + remaining += pre_rounds_placed - final_placed; // volume shed while rebalancing + total_placed = final_placed; + } + } + return { total_placed, remaining }; +} + +/* Placement core; see place_demand for the public contract. */ +std::pair FlowPolicy::place_demand_body(FlowGraph& fg, + NodeId src, NodeId dst, + FlowClass flowClass, + double volume, + std::optional target_per_flow, + std::optional min_flow) { + const bool is_static = has_static_paths(); + // Compute target flow per flow-record. // target: the volume to place per flow (or globally if target_per_flow is unset). // per_target: refined target for EqualBalanced mode (considers source capacity). double target = target_per_flow.value_or(volume); double per_target = target; - // For EqualBalanced mode with max_flow_count, compute a per-flow target based on - // available source capacity and the requested volume, divided by the number of flows. - if (flow_placement_ == FlowPlacement::EqualBalanced && max_flow_count_.has_value()) { + // EqualBalanced divisor: the number of flows the volume is split across. For a + // pinned policy that is the number of USABLE bundles (a head-end hashes over up + // LSPs only); dynamically it is the configured max_flow_count. + int eb_divisor = 0; + if (flow_placement_ == FlowPlacement::EqualBalanced) { + if (is_static) eb_divisor = static_cast(static_bundles_.size()); + else if (max_flow_count_.has_value()) eb_divisor = *max_flow_count_; + } + + // For EqualBalanced, compute a per-flow target based on available source + // capacity and the requested volume, divided by the number of flows. + if (eb_divisor > 0) { const auto& g = fg.graph(); auto row = g.row_offsets_view(); auto aei = g.adj_edge_index_view(); @@ -226,27 +327,24 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, src_cap += static_cast(residual[eid]); } } - // Compute per-flow target as the minimum of: - // - requested volume / max_flow_count - // - source capacity / max_flow_count - double per_req = target / static_cast(*max_flow_count_); - double per_src = src_cap / static_cast(*max_flow_count_); + // Per-flow target: min of requested volume and source capacity, per flow. + double per_req = target / static_cast(eb_divisor); + double per_src = src_cap / static_cast(eb_divisor); per_target = std::max(kMinFlow, std::min(per_req, per_src)); } // Initialize flows if none exist yet. if (flows_.empty()) { - if (!static_paths_.empty()) { - // Static paths: create one flow per static path. - if (max_flow_count_.has_value() && static_cast(static_paths_.size()) != *max_flow_count_) { - throw std::invalid_argument("If set, max_flow_count must be equal to the number of static paths."); + if (is_static) { + // Pinned bundles: one flow per usable bundle, in supply order, each bound to + // its own (pruned) DAG and post-prune cost. + if (src != static_src_ || dst != static_dst_) { + throw std::invalid_argument( + "Source and destination nodes of static paths do not match demand."); } - for (auto const& t : static_paths_) { - if (std::get<0>(t) == src && std::get<1>(t) == dst) { - [[maybe_unused]] auto* created = create_flow(fg, src, dst, flowClass, std::nullopt); - } else { - throw std::invalid_argument("Source and destination nodes of static paths do not match demand."); - } + for (auto const& b : static_bundles_) { + FlowIndex idx{src, dst, flowClass, next_flow_id_++}; + flows_.emplace(idx, FlowRecord(idx, src, dst, b.dag, b.cost)); } } else { // Dynamic paths: seed initial flows. @@ -254,17 +352,33 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, if (max_flow_count_.has_value()) { initial = std::min(initial, *max_flow_count_); } - for (int i=0;i(per_target) - : min_flow; - [[maybe_unused]] auto* created = create_flow(fg, src, dst, flowClass, min_req); + auto min_req = (flow_placement_ == FlowPlacement::EqualBalanced && max_flow_count_.has_value()) + ? std::optional(per_target) + : min_flow; + // Seeding places no flow, so residuals do not change between iterations and + // every create_flow() here would recompute the identical SPF. Compute the + // bundle once and copy it into each seeded flow. + if (initial > 0) { + if (auto pb = get_path_bundle(fg, src, dst, min_req)) { + for (int i = 0; i < initial; ++i) { + FlowIndex idx{src, dst, flowClass, next_flow_id_++}; + FlowRecord f(idx, src, dst, pb->first, pb->second); + flows_.emplace(idx, std::move(f)); + } + } } } } // Round-robin placement: iterate over flows and try to place volume on each. std::deque q; for (auto const& kv : flows_) q.push_back(kv.first); + if (is_static) { + // Pinned bundles have a documented precedence: bundle-supply order. Flow ids + // are assigned in that order, so sorting makes the visit order deterministic + // across platforms (unordered_map iteration order is not). + std::sort(q.begin(), q.end(), + [](const FlowIndex& a, const FlowIndex& b) { return a.flowId < b.flowId; }); + } double total_placed = 0.0; int no_progress = 0; // counter for consecutive iterations with no progress int iters = 0; @@ -290,7 +404,7 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, // For multipath flows, this tracks saturated edges within the DAG. // For tunnel flows, this allows different tunnels to discover different paths // as residuals change, enabling natural fan-out across equal-cost paths. - if (flow_placement_ == FlowPlacement::EqualBalanced && static_paths_.empty()) { + if (flow_placement_ == FlowPlacement::EqualBalanced && !is_static) { if (auto pb = get_path_bundle(fg, f->src, f->dst, std::optional(per_target))) { f->dag = std::move(pb->first); f->cost = pb->second; @@ -300,7 +414,7 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, if (target_per_flow.has_value()) { // When a per-flow target is specified (e.g., during rebalancing), cap by remaining per-flow target. need = std::max(0.0, target - f->placed_flow); - } else if (flow_placement_ == FlowPlacement::EqualBalanced && max_flow_count_.has_value()) { + } else if (eb_divisor > 0) { // For EqualBalanced, request only the remaining deficit toward per-target for this flow. need = std::max(0.0, per_target - f->placed_flow); } else { @@ -317,8 +431,11 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, if (shortest_path_) { break; } - // track recent placements - if (diminishing_returns_enabled_) { + // Track recent placements. For a pinned policy, arm the window only once every + // flow has been visited: with more bundles than the window, the early rounds of + // small per-flow placements must not starve the bundles not yet visited. + if (diminishing_returns_enabled_ && + (!is_static || iters >= static_cast(flows_.size()))) { recent.push_back(placed); if (static_cast(recent.size()) > diminishing_returns_window_) recent.pop_front(); if (static_cast(recent.size()) == diminishing_returns_window_) { @@ -334,52 +451,39 @@ std::pair FlowPolicy::place_demand(FlowGraph& fg, } else { no_progress = 0; } - if (flow_placement_ == FlowPlacement::EqualBalanced) { - if (max_flow_count_.has_value()) { - // Bounded EB: add flows up to configured maximum. - if (static_cast(flows_.size()) < *max_flow_count_) { - if (auto* nf = create_flow(fg, src, dst, flowClass, std::optional(per_target))) q.push_back(nf->index); + // A pinned policy neither grows its flow set nor reoptimizes: the pinned-ness + // guard is explicit, never inferred from flow-count arithmetic. + if (!is_static) { + if (flow_placement_ == FlowPlacement::EqualBalanced) { + if (max_flow_count_.has_value()) { + // Bounded EB: add flows up to configured maximum. + if (static_cast(flows_.size()) < *max_flow_count_) { + if (auto* nf = create_flow(fg, src, dst, flowClass, std::optional(per_target))) q.push_back(nf->index); + } + } else { + // Unbounded EB: rely on a single flow to equalize over the DAG. + // Do not create additional flows implicitly. } } else { - // Unbounded EB: rely on a single flow to equalize over the DAG. - // Do not create additional flows implicitly. - } - } else { - if (target - f->placed_flow >= kMinFlow) { - if (!max_flow_count_ || static_cast(flows_.size()) < *max_flow_count_) { - if (auto* nf = create_flow(fg, src, dst, flowClass, std::nullopt)) q.push_back(nf->index); - } else { - if (auto* rf = reoptimize_flow(fg, f->index, kMinFlow)) q.push_back(rf->index); + if (target - f->placed_flow >= kMinFlow) { + if (!max_flow_count_ || static_cast(flows_.size()) < *max_flow_count_) { + if (auto* nf = create_flow(fg, src, dst, flowClass, std::nullopt)) q.push_back(nf->index); + } else { + if (auto* rf = reoptimize_flow(fg, f->index, kMinFlow)) q.push_back(rf->index); + } } } } if (iters >= max_total_iterations_) break; } - // Reoptimize all flows after placement if enabled + // Reoptimize all flows after placement if enabled (no-op for pinned policies: + // reoptimize_flow returns immediately when static paths are configured). if (reoptimize_flows_on_each_placement_) { for (auto& kv : flows_) { (void)reoptimize_flow(fg, kv.first, kMinFlow); } } - - // For EQUAL_BALANCED placement, rebalance flows to maintain equal volumes. - if (flow_placement_ == FlowPlacement::EqualBalanced && !flows_.empty()) { - double target_eq = placed_demand() / static_cast(flows_.size()); - bool unbalanced = false; - for (auto const& kv : flows_) { - if (std::abs(target_eq - kv.second.placed_flow) >= kMinFlow) { unbalanced = true; break; } - } - if (unbalanced) { - bool prev_reopt = reoptimize_flows_on_each_placement_; - reoptimize_flows_on_each_placement_ = false; - auto pr = rebalance_demand(fg, src, dst, flowClass, target_eq); - // pr.first = placed in rebalanced pass, pr.second = excess - volume += pr.second; // leave remaining volume - reoptimize_flows_on_each_placement_ = prev_reopt; - total_placed = placed_demand(); - } - } return { total_placed, volume }; } @@ -389,6 +493,10 @@ std::pair FlowPolicy::rebalance_demand(FlowGraph& fg, NodeId src, NodeId dst, FlowClass flowClass, double target_per_flow) { + // Must run before remove_demand() empties flows_, or the check inside + // place_demand() would have nothing left to compare against and would + // silently retarget this policy's volume onto a different node pair. + check_demand_target(fg, src, dst, "rebalance_demand"); double vol = placed_demand(); remove_demand(fg); return place_demand(fg, src, dst, flowClass, vol, target_per_flow, std::nullopt); @@ -404,16 +512,253 @@ void FlowPolicy::remove_demand(FlowGraph& fg) { best_path_cost_ = std::numeric_limits::max(); } -/* Configure static paths to be used instead of dynamic SPF selection. If - `max_flow_count` is not set, it is set to the number of provided paths. */ -void FlowPolicy::set_static_paths(std::vector> paths) { - static_paths_ = std::move(paths); - if (max_flow_count_.has_value() && static_cast(static_paths_.size()) != *max_flow_count_) { +namespace { + +/* Validate one pinned bundle against the graph, prune it against the policy's + masks, and compute its post-prune min cost. Throws std::invalid_argument on a + malformed bundle; returns nullopt for a structurally valid bundle that has no + surviving src->dst walk under the masks (a DOWN LSP). */ +std::optional> +prepare_static_bundle(const StrictMultiDiGraph& g, const PredDAG& dag, + NodeId src, NodeId dst, + std::span node_mask, + std::span edge_mask) { + const auto N = static_cast(g.num_nodes()); + const auto E = static_cast(g.num_edges()); + const auto& off = dag.parent_offsets; + const auto& par = dag.parents; + const auto& via = dag.via_edges; + + // Shape: CSR offsets over N nodes, entry arrays sized by the final offset. + if (off.size() != N + 1 || off.front() != 0) { + throw std::invalid_argument("static path bundle: parent_offsets must have length num_nodes + 1 and start at 0"); + } + for (std::size_t v = 0; v + 1 < off.size(); ++v) { + if (off[v] > off[v + 1]) { + throw std::invalid_argument("static path bundle: parent_offsets must be non-decreasing"); + } + } + const auto entries = static_cast(off.back()); + if (par.size() != entries || via.size() != entries) { + throw std::invalid_argument("static path bundle: parents/via_edges size must equal parent_offsets.back()"); + } + + // Entries: ids in range, and each via edge must actually connect parent -> child + // in THIS graph. Without the endpoint check, a well-shaped DAG built for a + // different graph (or with shuffled edge ids) would silently place flow on + // arbitrary edges. + const auto esrc = g.edge_src_view(); + const auto edst = g.edge_dst_view(); + for (std::size_t v = 0; v < N; ++v) { + for (auto i = static_cast(off[v]); i < static_cast(off[v + 1]); ++i) { + const auto pnode = par[i]; + const auto e = via[i]; + if (pnode < 0 || static_cast(pnode) >= N) { + throw std::invalid_argument("static path bundle: parent node id out of range"); + } + if (e < 0 || static_cast(e) >= E) { + throw std::invalid_argument("static path bundle: via edge id out of range"); + } + if (esrc[static_cast(e)] != pnode || + edst[static_cast(e)] != static_cast(v)) { + throw std::invalid_argument( + "static path bundle: via edge does not connect its parent to its node in " + "this graph (bundle built for a different graph?)"); + } + } + } + + // Acyclicity (Kahn over parent -> child entries). + { + std::vector indeg(N, 0); + for (std::size_t v = 0; v < N; ++v) { + indeg[v] = static_cast(off[v + 1] - off[v]); + } + std::vector stack; + stack.reserve(N); + for (std::size_t v = 0; v < N; ++v) { + if (indeg[v] == 0) stack.push_back(static_cast(v)); + } + std::size_t seen = 0; + // Child adjacency: parent -> list of children, derived on the fly. + std::vector> children(N); + for (std::size_t v = 0; v < N; ++v) { + for (auto i = static_cast(off[v]); i < static_cast(off[v + 1]); ++i) { + children[static_cast(par[i])].push_back(static_cast(v)); + } + } + while (!stack.empty()) { + auto u = stack.back(); stack.pop_back(); + ++seen; + for (auto v : children[static_cast(u)]) { + if (--indeg[static_cast(v)] == 0) stack.push_back(v); + } + } + if (seen != N) { + throw std::invalid_argument("static path bundle: predecessor structure contains a cycle"); + } + } + + // Structural src->dst connectivity ignoring masks: a bundle that never had an + // src->dst walk is malformed for this demand, distinct from being down. + auto backward_reaches_src = [&](auto&& admit_entry) { + std::vector reach(N, 0); + std::vector bfs; + bfs.push_back(dst); + reach[static_cast(dst)] = 1; + for (std::size_t head = 0; head < bfs.size(); ++head) { + const auto v = static_cast(bfs[head]); + for (auto i = static_cast(off[v]); i < static_cast(off[v + 1]); ++i) { + if (!admit_entry(i)) continue; + const auto pnode = par[i]; + if (!reach[static_cast(pnode)]) { + reach[static_cast(pnode)] = 1; + bfs.push_back(pnode); + } + } + } + return reach[static_cast(src)] != 0; + }; + if (!backward_reaches_src([](std::size_t) { return true; })) { + throw std::invalid_argument("static path bundle: no src->dst walk exists in the bundle"); + } + + // Prune against the policy's masks (failure exclusions), then re-check + // connectivity: no surviving walk means the LSP is DOWN. + const bool use_nm = !node_mask.empty(); + const bool use_em = !edge_mask.empty(); + auto entry_up = [&](std::size_t i) { + const auto pnode = static_cast(par[i]); + const auto e = static_cast(via[i]); + if (use_em && !edge_mask[e]) return false; + if (use_nm && !node_mask[pnode]) return false; + return true; + }; + auto node_up = [&](NodeId v) { return !use_nm || node_mask[static_cast(v)]; }; + if (!node_up(src) || !node_up(dst)) return std::nullopt; + + PredDAG pruned; + pruned.parent_offsets.assign(N + 1, 0); + for (std::size_t v = 0; v < N; ++v) { + std::int32_t c = 0; + if (node_up(static_cast(v))) { + for (auto i = static_cast(off[v]); i < static_cast(off[v + 1]); ++i) { + if (entry_up(i)) ++c; + } + } + pruned.parent_offsets[v + 1] = pruned.parent_offsets[v] + c; + } + pruned.parents.resize(static_cast(pruned.parent_offsets.back())); + pruned.via_edges.resize(static_cast(pruned.parent_offsets.back())); + for (std::size_t v = 0; v < N; ++v) { + auto w = static_cast(pruned.parent_offsets[v]); + if (!node_up(static_cast(v))) continue; + for (auto i = static_cast(off[v]); i < static_cast(off[v + 1]); ++i) { + if (!entry_up(i)) continue; + pruned.parents[w] = par[i]; + pruned.via_edges[w] = via[i]; + ++w; + } + } + + const auto& poff = pruned.parent_offsets; + { + std::vector reach(N, 0); + std::vector bfs; + bfs.push_back(dst); + reach[static_cast(dst)] = 1; + for (std::size_t head = 0; head < bfs.size(); ++head) { + const auto v = static_cast(bfs[head]); + for (auto i = static_cast(poff[v]); i < static_cast(poff[v + 1]); ++i) { + const auto pnode = pruned.parents[i]; + if (!reach[static_cast(pnode)]) { + reach[static_cast(pnode)] = 1; + bfs.push_back(pnode); + } + } + } + if (!reach[static_cast(src)]) return std::nullopt; // DOWN + } + + // Cost: min-cost src->dst walk within the PRUNED bundle, so the flow reports the + // metric of a walk it can actually use. DP in a topological order of the pruned + // DAG (acyclicity established above; pruning cannot introduce cycles). + Cost best = std::numeric_limits::max(); + { + const auto costs = g.cost_view(); + std::vector dist(N, std::numeric_limits::max()); + dist[static_cast(src)] = 0; + std::vector indeg(N, 0); + std::vector>> children(N); + for (std::size_t v = 0; v < N; ++v) { + indeg[v] = static_cast(poff[v + 1] - poff[v]); + for (auto i = static_cast(poff[v]); i < static_cast(poff[v + 1]); ++i) { + children[static_cast(pruned.parents[i])].emplace_back( + static_cast(v), pruned.via_edges[i]); + } + } + std::vector stack; + for (std::size_t v = 0; v < N; ++v) { + if (indeg[v] == 0) stack.push_back(static_cast(v)); + } + while (!stack.empty()) { + auto u = stack.back(); stack.pop_back(); + const auto du = dist[static_cast(u)]; + for (auto [v, e] : children[static_cast(u)]) { + if (du != std::numeric_limits::max()) { + const Cost cand = du + costs[static_cast(e)]; + if (cand < dist[static_cast(v)]) dist[static_cast(v)] = cand; + } + if (--indeg[static_cast(v)] == 0) stack.push_back(v); + } + } + best = dist[static_cast(dst)]; + } + return std::make_optional(std::make_pair(std::move(pruned), best)); +} + +} // namespace + +/* See flow_policy.hpp for the contract. */ +void FlowPolicy::set_static_paths(NodeId src, NodeId dst, std::vector bundles) { + const auto* g = ctx_.graph.graph.get(); + if (g == nullptr) { + throw std::invalid_argument("FlowPolicy::set_static_paths: policy has no graph"); + } + if (!flows_.empty()) { + throw std::invalid_argument( + "FlowPolicy::set_static_paths: policy already holds flows; call remove_demand() first"); + } + if (bundles.empty()) { + throw std::invalid_argument("FlowPolicy::set_static_paths: bundles must be non-empty"); + } + if (shortest_path_) { + throw std::invalid_argument( + "FlowPolicy::set_static_paths: incompatible with shortest_path=true (single-" + "augmentation IP semantics contradict pinned multi-LSP placement)"); + } + const auto N = g->num_nodes(); + if (src < 0 || src >= N || dst < 0 || dst >= N || src == dst) { + throw std::invalid_argument("FlowPolicy::set_static_paths: src/dst out of range or equal"); + } + // Validate the USER's max_flow_count against the supplied bundle count. The + // configured value is never overwritten: the usable (up) count U drives the + // per-flow math at placement time, and N - flow_count() is the down-LSP count. + if (max_flow_count_.has_value() && *max_flow_count_ != static_cast(bundles.size())) { throw std::invalid_argument("If set, max_flow_count must be equal to the number of static paths."); } - if (!max_flow_count_.has_value()) { - max_flow_count_ = static_cast(static_paths_.size()); + + std::vector usable; + usable.reserve(bundles.size()); + for (auto const& dag : bundles) { + if (auto prepared = prepare_static_bundle(*g, dag, src, dst, node_mask_, edge_mask_)) { + usable.push_back(StaticBundle{std::move(prepared->first), prepared->second}); + } } + static_src_ = src; + static_dst_ = dst; + static_supplied_count_ = static_cast(bundles.size()); + static_bundles_ = std::move(usable); } } // namespace netgraph::core diff --git a/src/flow_state.cpp b/src/flow_state.cpp index bdb39da..3789853 100644 --- a/src/flow_state.cpp +++ b/src/flow_state.cpp @@ -19,9 +19,8 @@ #include #include #include -#include #include -#include +#include #include #include #include @@ -98,68 +97,99 @@ struct FlowWorkspace { struct EdgeGroup { std::int32_t from; // child v (destination of grouped edges) std::int32_t to; // parent u (source of grouped edges) - std::vector eids; // underlying forward edges u->v (may be multiple parallel edges) + // Underlying forward edges u->v (parallel edges) as a slice of GroupSet::eids. + // Storing a slice rather than a std::vector keeps group building allocation-free; + // rebuilding groups after every augmentation used to dominate place_on_dag. + std::int32_t eid_begin {0}; + std::int32_t eid_count {0}; Cap sum_cap {0.0}; // sum of residual capacities for Proportional placement Cap min_cap {0.0}; // min residual capacity for EqualBalanced placement }; +// Groups plus the arena backing their edge slices, and the scratch used to build +// them. One instance is reused across every rebuild within a place_on_dag call so +// the buffers keep their capacity. +struct GroupSet { + std::vector groups; + std::vector eids; // arena: group gi owns [eid_begin, +eid_count) + std::vector reach; // scratch: backward reachability from t + std::vector uniq; // scratch: distinct parents of one node + std::vector bfs; // scratch: BFS queue + + [[nodiscard]] std::span edges_of(const EdgeGroup& gr) const noexcept { + return std::span(eids.data() + gr.eid_begin, + static_cast(gr.eid_count)); + } +}; + // Build grouped edges by (parent u, child v) that can reach destination t, // using the current residual snapshot. -static std::vector build_groups_residual(const StrictMultiDiGraph& g, - const PredDAG& dag, NodeId t, - const std::vector& residual) { - std::vector groups; +static void build_groups_residual(const StrictMultiDiGraph& g, + const PredDAG& dag, NodeId t, + const std::vector& residual, + GroupSet& gs) { + gs.groups.clear(); + gs.eids.clear(); const auto& offsets = dag.parent_offsets; const auto& parents = dag.parents; const auto& via = dag.via_edges; const auto N = g.num_nodes(); // Compute reachability: BFS backward from destination t to identify nodes on SPF DAG. - std::vector reach(static_cast(N), 0); + gs.reach.assign(static_cast(N), 0); if (t >= 0 && t < N) { - std::queue q; q.push(t); reach[static_cast(t)] = 1; - while (!q.empty()) { - auto v = q.front(); q.pop(); + gs.bfs.clear(); + gs.bfs.push_back(t); + gs.reach[static_cast(t)] = 1; + for (std::size_t head = 0; head < gs.bfs.size(); ++head) { + const auto v = gs.bfs[head]; // Iterate over v's predecessors (parents in the DAG). std::size_t s = static_cast(offsets[static_cast(v)]); std::size_t e = static_cast(offsets[static_cast(v + 1)]); for (std::size_t i = s; i < e; ++i) { auto u = parents[i]; - if (!reach[static_cast(u)]) { reach[static_cast(u)] = 1; q.push(u); } + if (!gs.reach[static_cast(u)]) { gs.reach[static_cast(u)] = 1; gs.bfs.push_back(u); } } } } // For each reachable node v, group its incoming DAG edges by parent node u. // This creates one group per (u, v) pair, aggregating parallel edges. + // Two passes per node (collect distinct parents, then gather each parent's edges) + // keep every group's edges contiguous in the arena without any allocation. for (std::int32_t v = 0; v < N; ++v) { - if (!reach[static_cast(v)]) continue; - // Small linear grouping by parent (faster than a hash for typical degrees). - std::vector>> by_parent; + if (!gs.reach[static_cast(v)]) continue; const std::size_t s = static_cast(offsets[static_cast(v)]); const std::size_t e = static_cast(offsets[static_cast(v + 1)]); + // Pass 1: distinct parents, in first-appearance order (matches the previous + // by_parent ordering, so emitted groups keep the same order as before). + gs.uniq.clear(); for (std::size_t i = s; i < e; ++i) { const auto u = parents[i]; bool found = false; - for (auto& pr : by_parent) { - if (pr.first == u) { pr.second.push_back(via[i]); found = true; break; } - } - if (!found) by_parent.emplace_back(u, std::vector{ via[i] }); + for (auto pu : gs.uniq) { if (pu == u) { found = true; break; } } + if (!found) gs.uniq.push_back(u); } - for (auto& kv : by_parent) { - EdgeGroup gr; gr.from = v; gr.to = kv.first; gr.eids.clear(); + // Pass 2: gather each parent's admissible parallel edges contiguously. + for (auto u : gs.uniq) { + EdgeGroup gr; + gr.from = v; gr.to = u; + gr.eid_begin = static_cast(gs.eids.size()); gr.sum_cap = static_cast(0.0); gr.min_cap = std::numeric_limits::infinity(); - for (auto eid0 : kv.second) { + for (std::size_t i = s; i < e; ++i) { + if (parents[i] != u) continue; + const auto eid0 = via[i]; const Cap c = residual[static_cast(eid0)]; if (c >= kMinCap) { - gr.eids.push_back(eid0); + gs.eids.push_back(eid0); gr.sum_cap += c; gr.min_cap = std::min(gr.min_cap, c); } } + gr.eid_count = static_cast(gs.eids.size()) - gr.eid_begin; if (gr.min_cap == std::numeric_limits::infinity()) gr.min_cap = static_cast(0.0); - if (!gr.eids.empty()) groups.push_back(std::move(gr)); + if (gr.eid_count > 0) gs.groups.push_back(gr); + // else: nothing was appended, so the arena is already back at eid_begin. } } - return groups; } // Construct reversed residual graph for Dinic BFS/DFS using group capacities. @@ -216,8 +246,11 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, const auto N = g_->num_nodes(); if (src < 0 || src >= N || dst < 0 || dst >= N || src == dst) return 0.0; - // Build groups using current residual - auto groups = build_groups_residual(*g_, dag, dst, residual_); + // Build groups using current residual. `gs` is reused across rebuilds so its + // buffers keep their capacity for the whole call. + GroupSet gs; + build_groups_residual(*g_, dag, dst, residual_, gs); + const auto& groups = gs.groups; Flow placed = static_cast(0.0); double remaining = static_cast(requested_flow); @@ -251,7 +284,7 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, // Guard against division by zero when group capacity is numerically zero. double denom = gr.sum_cap > kMinCap ? gr.sum_cap : 1.0; // Proportional split: each edge gets share = sent * (edge_residual / sum_residual). - for (auto eid : gr.eids) { + for (auto eid : gs.edges_of(gr)) { Cap base = residual_[static_cast(eid)]; double share = sent * (static_cast(base) / denom); edge_flow_[static_cast(eid)] += static_cast(share); @@ -261,7 +294,7 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, } } // Rebuild groups for next tier using updated residual - groups = build_groups_residual(*g_, dag, dst, residual_); + build_groups_residual(*g_, dag, dst, residual_, gs); build_reversed_residual(ws, N, groups); } } else { @@ -274,9 +307,9 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, std::vector rev_cap(groups.size(), 0.0); for (std::size_t gi = 0; gi < groups.size(); ++gi) { const auto& gr = groups[gi]; - if (gr.eids.empty()) continue; + if (gr.eid_count == 0) continue; // EB: group admissible total = min_edge_residual * |edges| - const double cap_rev = static_cast(gr.min_cap) * static_cast(gr.eids.size()); + const double cap_rev = static_cast(gr.min_cap) * static_cast(gr.eid_count); if (cap_rev >= kMinCap) { succ[static_cast(gr.to)].push_back(gi); // u -> v (group index) rev_cap[gi] = cap_rev; @@ -301,7 +334,7 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, for (std::size_t u = 0; u < succ.size(); ++u) { if (!reach[u]) continue; int s = 0; - for (auto gi : succ[u]) s += static_cast(groups[gi].eids.size()); + for (auto gi : succ[u]) s += static_cast(groups[gi].eid_count); node_split[u] = s; } @@ -332,9 +365,9 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, if (split <= 0) continue; for (auto gi : succ[static_cast(u)]) { const auto& gr = groups[gi]; - if (gr.eids.empty()) continue; + if (gr.eid_count == 0) continue; // Group share proportional to number of edges (equal per-edge split). - double push = f_in * (static_cast(gr.eids.size()) / static_cast(split)); + double push = f_in * (static_cast(gr.eid_count) / static_cast(split)); if (push < kEpsilon) continue; assigned[gi] += push; auto v = static_cast(gr.from); @@ -371,11 +404,11 @@ Flow FlowState::place_on_dag(NodeId src, NodeId dst, const PredDAG& dag, placed += use; // Apply scaled group assignments equally over parallel edges. for (std::size_t gi = 0; gi < groups.size(); ++gi) { - const auto& gr = groups[gi]; if (gr.eids.empty()) continue; + const auto& gr = groups[gi]; if (gr.eid_count == 0) continue; double flow_scaled = assigned[gi] * static_cast(use); if (flow_scaled < kMinFlow) continue; - double per_edge = flow_scaled / static_cast(gr.eids.size()); - for (auto eid : gr.eids) { + double per_edge = flow_scaled / static_cast(gr.eid_count); + for (auto eid : gs.edges_of(gr)) { edge_flow_[static_cast(eid)] += static_cast(per_edge); double base = static_cast(residual_[static_cast(eid)]); residual_[static_cast(eid)] = static_cast(std::max(0.0, base - per_edge)); @@ -481,8 +514,12 @@ MinCut FlowState::compute_min_cut(NodeId src, std::span node_mask, s auto eid = static_cast(in_aei[j]); if (use_edge_mask && !edge_mask[eid]) continue; if (use_node_mask && !node_mask[static_cast(w)]) continue; - double flow_e = g_->capacity_view()[eid] - residual_[eid]; - if (flow_e > kMinFlow && !visited[static_cast(w)]) { + // Reverse arcs exist where THIS state has placed cancellable flow. Using + // edge_flow_ (not capacity - residual) keeps the traversal correct when + // the state was constructed with a custom residual_init, where + // capacity - residual also counts pre-existing usage this state never + // placed and cannot cancel. + if (edge_flow_[eid] > kMinFlow && !visited[static_cast(w)]) { visited[static_cast(w)] = 1; q.push(w); } diff --git a/src/k_shortest_paths.cpp b/src/k_shortest_paths.cpp index 6cb5b62..ce410e6 100644 --- a/src/k_shortest_paths.cpp +++ b/src/k_shortest_paths.cpp @@ -7,7 +7,6 @@ #include "netgraph/core/k_shortest_paths.hpp" #include -#include #include #include #include @@ -36,6 +35,13 @@ struct Candidate { bool operator>(const Candidate& o) const { return cost > o.cost; } }; +struct VectorHash { size_t operator()(const std::vector& v) const noexcept { + size_t h = 1469598103934665603ull; + for (auto x : v) { h ^= static_cast(x + 0x9e3779b97f4a7c15ull); h *= 1099511628211ull; } + return h; +}}; +using PathSet = std::unordered_set, VectorHash>; + static std::optional dijkstra_single(const StrictMultiDiGraph& g, NodeId s, NodeId t, const std::vector* node_mask, @@ -107,9 +113,15 @@ static std::optional dijkstra_single(const StrictMultiDiGraph& g, NodeId s return p; } -// Enumerate all shortest spur paths from spur->t using PredDAG produced by -// shortest_paths(spur, ..., multipath=true). Returns a list of (nodes, edges) -// sequences in forward order. +// Enumerate shortest spur paths from spur->t using the PredDAG produced by +// shortest_paths(spur, ..., multipath=true), in deterministic DFS order. +// All spur paths in the DAG share the same (shortest) cost, and at most +// max_count candidates can ever be accepted downstream, so enumeration stops +// after emitting max_count paths. Without the bound this walk materializes +// every equal-cost path — exponential in ECMP fan-out (2^k paths on a k-stage +// ladder). When dedup is supplied (unique mode), paths whose full edge +// sequence (prefix + spur segment) was already visited are skipped without +// consuming the bound, so the cap cannot starve fresh candidates. static void dfs_spur_paths(NodeId spur, NodeId v, const std::vector& off, const std::vector& parents, @@ -117,7 +129,11 @@ static void dfs_spur_paths(NodeId spur, NodeId v, std::vector& nodes_rev, std::vector& edges_rev, std::vector>& out_nodes, - std::vector>& out_edges) { + std::vector>& out_edges, + std::size_t max_count, + const std::vector* prefix_edges, + const PathSet* dedup) { + if (out_nodes.size() >= max_count) return; if (v == spur) { // build forward std::vector n; n.reserve(nodes_rev.size() + 1); @@ -125,6 +141,13 @@ static void dfs_spur_paths(NodeId spur, NodeId v, for (auto it = nodes_rev.rbegin(); it != nodes_rev.rend(); ++it) n.push_back(*it); std::vector e; e.reserve(edges_rev.size()); for (auto it = edges_rev.rbegin(); it != edges_rev.rend(); ++it) e.push_back(*it); + if (dedup != nullptr) { + std::vector full; + full.reserve(prefix_edges->size() + e.size()); + full.insert(full.end(), prefix_edges->begin(), prefix_edges->end()); + full.insert(full.end(), e.begin(), e.end()); + if (dedup->find(full) != dedup->end()) return; // already visited; free the slot + } out_nodes.push_back(std::move(n)); out_edges.push_back(std::move(e)); return; @@ -132,11 +155,13 @@ static void dfs_spur_paths(NodeId spur, NodeId v, auto s = off[static_cast(v)]; auto e = off[static_cast(v) + 1]; for (std::int32_t i = s; i < e; ++i) { + if (out_nodes.size() >= max_count) return; auto u = parents[static_cast(i)]; auto eid = via[static_cast(i)]; edges_rev.push_back(static_cast(eid)); nodes_rev.push_back(v); - dfs_spur_paths(spur, u, off, parents, via, nodes_rev, edges_rev, out_nodes, out_edges); + dfs_spur_paths(spur, u, off, parents, via, nodes_rev, edges_rev, out_nodes, out_edges, + max_count, prefix_edges, dedup); nodes_rev.pop_back(); edges_rev.pop_back(); } @@ -184,33 +209,17 @@ std::vector, PredDAG>> k_shortest_paths( max_cost = best_cost * (*max_cost_factor); } if (p0->cost <= max_cost) paths.push_back(*p0); + // A max_cost_factor below 1.0 puts the ceiling under the shortest path itself, so + // nothing was admitted. Return now: the k > 1 loop below opens with paths.back(), + // which is undefined behaviour on an empty vector. (The k == 1 branch would have + // masked this, so the guard must sit ahead of it.) + if (paths.empty()) return {}; if (k == 1) { // Convert and return std::vector, PredDAG>> items; items.reserve(paths.size()); - auto cost_view = g.cost_view(); for (auto const& P : paths) { - std::vector dist(static_cast(g.num_nodes()), std::numeric_limits::max()); - PredDAG dag; - dag.parent_offsets.assign(static_cast(g.num_nodes() + 1), 0); - // Fill distances along path and one-parent predecessors - if (!P.nodes.empty()) { - dist[static_cast(P.nodes.front())] = 0; - for (std::size_t i = 1; i < P.nodes.size(); ++i) { - auto u = P.nodes[i-1]; auto v = P.nodes[i]; auto e = P.edges[i-1]; - dist[static_cast(v)] = dist[static_cast(u)] + static_cast(cost_view[static_cast(e)]); - dag.parent_offsets[static_cast(v+1)] = 1; - } - for (std::size_t v = 1; v < dag.parent_offsets.size(); ++v) dag.parent_offsets[v] += dag.parent_offsets[v-1]; - dag.parents.resize(static_cast(dag.parent_offsets.back())); - dag.via_edges.resize(static_cast(dag.parent_offsets.back())); - for (std::size_t i = 1; i < P.nodes.size(); ++i) { - auto v = P.nodes[i]; - auto base = static_cast(dag.parent_offsets[static_cast(v)]); - dag.parents[base] = P.nodes[i-1]; dag.via_edges[base] = P.edges[i-1]; - } - } - items.emplace_back(std::move(dist), std::move(dag)); + items.push_back(path_to_pred_dag(g, P.nodes, P.edges)); } return items; } @@ -219,12 +228,7 @@ std::vector, PredDAG>> k_shortest_paths( auto cost_view = g.cost_view(); std::priority_queue, std::greater> B; auto path_signature = [&](const std::vector& edges){ return edges; }; - struct VectorHash { size_t operator()(const std::vector& v) const noexcept { - size_t h = 1469598103934665603ull; - for (auto x : v) { h ^= static_cast(x + 0x9e3779b97f4a7c15ull); h *= 1099511628211ull; } - return h; - }}; - std::unordered_set, VectorHash> visited; + PathSet visited; visited.insert(path_signature(p0->edges)); for (int i = 1; i < k; ++i) { @@ -282,12 +286,23 @@ std::vector, PredDAG>> k_shortest_paths( dag_spur.parent_offsets[static_cast(dst)] == dag_spur.parent_offsets[static_cast(dst + 1)]) { continue; } + // Every spur path in the DAG costs dist_spur[dst]; skip the whole family + // if the resulting candidates would exceed the cost ceiling. + if (static_cast(prefix_cost[j]) + static_cast(dist_spur[static_cast(dst)]) > max_cost) { + continue; + } + // At most (k - accepted) further paths can ever be accepted, so that many + // fresh candidates from this spur family suffice. + const std::size_t want = static_cast(k) - paths.size(); + std::vector prefix_edges(last.edges.begin(), + last.edges.begin() + static_cast(j)); std::vector nodes_rev; std::vector edges_rev; std::vector> spur_nodes_list; std::vector> spur_edges_list; dfs_spur_paths(spur_node, dst, dag_spur.parent_offsets, dag_spur.parents, dag_spur.via_edges, - nodes_rev, edges_rev, spur_nodes_list, spur_edges_list); + nodes_rev, edges_rev, spur_nodes_list, spur_edges_list, + want, &prefix_edges, unique ? &visited : nullptr); for (std::size_t si = 0; si < spur_nodes_list.size(); ++si) { const auto& spur_nodes = spur_nodes_list[si]; const auto& spur_edges = spur_edges_list[si]; @@ -326,26 +341,7 @@ std::vector, PredDAG>> k_shortest_paths( std::vector, PredDAG>> items; items.reserve(paths.size()); for (auto const& P : paths) { - std::vector dist(static_cast(g.num_nodes()), std::numeric_limits::max()); - PredDAG dag; - dag.parent_offsets.assign(static_cast(g.num_nodes() + 1), 0); - if (!P.nodes.empty()) { - dist[static_cast(P.nodes.front())] = 0; - for (std::size_t i = 1; i < P.nodes.size(); ++i) { - auto u = P.nodes[i-1]; auto v = P.nodes[i]; auto e = P.edges[i-1]; - dist[static_cast(v)] = dist[static_cast(u)] + static_cast(cost_view[static_cast(e)]); - dag.parent_offsets[static_cast(v+1)] = 1; - } - for (std::size_t v = 1; v < dag.parent_offsets.size(); ++v) dag.parent_offsets[v] += dag.parent_offsets[v-1]; - dag.parents.resize(static_cast(dag.parent_offsets.back())); - dag.via_edges.resize(static_cast(dag.parent_offsets.back())); - for (std::size_t i = 1; i < P.nodes.size(); ++i) { - auto v = P.nodes[i]; - auto base = static_cast(dag.parent_offsets[static_cast(v)]); - dag.parents[base] = P.nodes[i-1]; dag.via_edges[base] = P.edges[i-1]; - } - } - items.emplace_back(std::move(dist), std::move(dag)); + items.push_back(path_to_pred_dag(g, P.nodes, P.edges)); } return items; } diff --git a/src/max_flow.cpp b/src/max_flow.cpp index a970a2f..81e3b29 100644 --- a/src/max_flow.cpp +++ b/src/max_flow.cpp @@ -14,6 +14,7 @@ #include "netgraph/core/constants.hpp" #include +#include #include #include #include @@ -23,7 +24,6 @@ #include #include #include -#include #include #include @@ -31,12 +31,12 @@ namespace netgraph::core { namespace { -std::size_t sensitivity_thread_budget(std::size_t candidate_count) { +std::size_t thread_budget_from_env(const char* env_name, std::size_t candidate_count) { if (candidate_count <= 1) { return 1; } - const char* env = std::getenv("NGRAPH_CORE_SENSITIVITY_THREADS"); + const char* env = std::getenv(env_name); if (env != nullptr && env[0] != '\0') { char* end = nullptr; unsigned long parsed = std::strtoul(env, &end, 10); @@ -151,6 +151,93 @@ calc_max_flow(const StrictMultiDiGraph& g, NodeId src, NodeId dst, if (shortest_path) break; } + // Completion phase. The tier loop above augments only along forward SPF DAGs + // and can never cancel an earlier placement, so it may terminate below the + // true maximum (its own min-cut then contradicts total_flow). Finish with + // BFS augmentation on the full residual graph, where traversing an edge + // backwards returns previously placed flow. Only meaningful for Proportional + // max-flow semantics: EqualBalanced models ECMP admission and + // require_capacity=false models fixed-cost IP routing, both of which are + // placement models rather than max-flow computations. + if (placement == FlowPlacement::Proportional && !shortest_path && require_capacity) { + const auto row = g.row_offsets_view(); + const auto col = g.col_indices_view(); + const auto aei = g.adj_edge_index_view(); + const auto irow = g.in_row_offsets_view(); + const auto icol = g.in_col_indices_view(); + const auto iaei = g.in_adj_edge_index_view(); + const auto costv = g.cost_view(); + const auto residual = fs.residual_view(); + const auto eflow = fs.edge_flow_view(); + std::vector prev_node(static_cast(N)); + std::vector prev_edge(static_cast(N)); + std::vector prev_fwd(static_cast(N)); + std::vector seen(static_cast(N)); + std::vector> fwd_deltas, rev_deltas; + while (true) { + std::fill(seen.begin(), seen.end(), 0); + std::queue bfs; + seen[static_cast(src)] = 1; + bfs.push(src); + bool found = false; + while (!bfs.empty() && !found) { + auto u = bfs.front(); bfs.pop(); + auto us = static_cast(u); + // Forward residual arcs u -> v. + for (auto j = static_cast(row[us]); j < static_cast(row[us + 1]); ++j) { + auto v = static_cast(col[j]); + auto eid = static_cast(aei[j]); + if (use_edge_mask && !edge_mask[eid]) continue; + if (use_node_mask && !node_mask[v]) continue; + if (seen[v] || residual[eid] <= kMinCap) continue; + seen[v] = 1; prev_node[v] = u; prev_edge[v] = static_cast(eid); prev_fwd[v] = 1; + if (static_cast(v) == dst) { found = true; break; } + bfs.push(static_cast(v)); + } + if (found) break; + // Reverse residual arcs u -> w along edges w -> u carrying flow. + for (auto j = static_cast(irow[us]); j < static_cast(irow[us + 1]); ++j) { + auto w = static_cast(icol[j]); + auto eid = static_cast(iaei[j]); + if (use_edge_mask && !edge_mask[eid]) continue; + if (use_node_mask && !node_mask[w]) continue; + if (seen[w] || eflow[eid] <= kMinFlow) continue; + seen[w] = 1; prev_node[w] = u; prev_edge[w] = static_cast(eid); prev_fwd[w] = 0; + if (static_cast(w) == dst) { found = true; break; } + bfs.push(static_cast(w)); + } + } + if (!found) break; + // Bottleneck and marginal path cost (forward costs minus cancelled costs). + double bottleneck = std::numeric_limits::infinity(); + Cost path_cost = 0; + for (auto v = dst; v != src; v = prev_node[static_cast(v)]) { + auto vs = static_cast(v); + auto eid = static_cast(prev_edge[vs]); + if (prev_fwd[vs]) { + bottleneck = std::min(bottleneck, static_cast(residual[eid])); + path_cost += costv[eid]; + } else { + bottleneck = std::min(bottleneck, static_cast(eflow[eid])); + path_cost -= costv[eid]; + } + } + if (!(bottleneck >= kMinFlow)) break; + fwd_deltas.clear(); rev_deltas.clear(); + for (auto v = dst; v != src; v = prev_node[static_cast(v)]) { + auto vs = static_cast(v); + if (prev_fwd[vs]) fwd_deltas.emplace_back(prev_edge[vs], static_cast(bottleneck)); + else rev_deltas.emplace_back(prev_edge[vs], static_cast(bottleneck)); + } + fs.apply_deltas(fwd_deltas, /*add=*/true); + fs.apply_deltas(rev_deltas, /*add=*/false); + total += static_cast(bottleneck); + bool merged2 = false; + for (auto& pr : cost_dist) { if (pr.first == path_cost) { pr.second += bottleneck; merged2 = true; break; } } + if (!merged2) cost_dist.emplace_back(path_cost, static_cast(bottleneck)); + } + } + summary.total_flow = total; if (with_edge_flows) { auto ef = fs.edge_flow_view(); @@ -177,7 +264,7 @@ calc_max_flow(const StrictMultiDiGraph& g, NodeId src, NodeId dst, if (with_reachable) { summary.reachable_nodes.assign(static_cast(g.num_nodes()), 0u); auto residual = fs.residual_view(); - auto capv = fs.capacity_view(); + auto eflows = fs.edge_flow_view(); const bool reach_use_node_mask = use_node_mask; const bool reach_use_edge_mask = use_edge_mask; const auto N = static_cast(g.num_nodes()); @@ -214,8 +301,7 @@ calc_max_flow(const StrictMultiDiGraph& g, NodeId src, NodeId dst, auto eid = static_cast(iae[p]); if (reach_use_edge_mask && !edge_mask[eid]) continue; if (reach_use_node_mask && !node_mask[u]) continue; - auto flow = capv[eid] - residual[eid]; - if (flow > kMinFlow && !summary.reachable_nodes[u]) { + if (eflows[eid] > kMinFlow && !summary.reachable_nodes[u]) { stack.push_back(static_cast(u)); } } @@ -247,21 +333,47 @@ batch_max_flow(const StrictMultiDiGraph& g, } } - std::vector out; - out.reserve(pairs.size()); - for (std::size_t i = 0; i < pairs.size(); ++i) { - auto pr = pairs[i]; - std::span nm = (i < node_masks.size() ? node_masks[i] : std::span{}); - std::span em = (i < edge_masks.size() ? edge_masks[i] : std::span{}); - auto [val, summary] = calc_max_flow(g, pr.first, pr.second, - placement, shortest_path, - require_capacity, - with_edge_flows, - with_reachable, - with_residuals, - nm, em); - out.push_back(std::move(summary)); + std::vector out(pairs.size()); + auto run_range = [&](std::size_t begin, std::size_t end) { + for (std::size_t i = begin; i < end; ++i) { + auto pr = pairs[i]; + std::span nm = (i < node_masks.size() ? node_masks[i] : std::span{}); + std::span em = (i < edge_masks.size() ? edge_masks[i] : std::span{}); + auto [val, summary] = calc_max_flow(g, pr.first, pr.second, + placement, shortest_path, + require_capacity, + with_edge_flows, + with_reachable, + with_residuals, + nm, em); + out[i] = std::move(summary); + } + }; + // Pairs are independent over an immutable graph; evaluate chunks in parallel. + // Thread count from NGRAPH_CORE_BATCH_THREADS env or hardware concurrency. + const auto thread_budget = thread_budget_from_env("NGRAPH_CORE_BATCH_THREADS", pairs.size()); + if (thread_budget <= 1) { + run_range(0, pairs.size()); + return out; + } + // Claim pairs one at a time from a shared counter rather than handing each worker a + // fixed contiguous chunk. Per-pair cost varies by orders of magnitude (a dense + // cross-fabric pair versus a disconnected one), so static chunking leaves workers + // idle whenever the expensive pairs happen to land in the same chunk. + std::atomic next_pair{0}; + auto run_dynamic = [&]() { + for (;;) { + const std::size_t i = next_pair.fetch_add(1, std::memory_order_relaxed); + if (i >= pairs.size()) break; + run_range(i, i + 1); + } + }; + std::vector> futures; + futures.reserve(thread_budget); + for (std::size_t w = 0; w < thread_budget; ++w) { + futures.emplace_back(std::async(std::launch::async, run_dynamic)); } + for (auto& f : futures) f.get(); return out; } @@ -312,7 +424,7 @@ sensitivity_analysis(const StrictMultiDiGraph& g, NodeId src, NodeId dst, results.reserve(candidates.size()); // Step 2: Iterate candidates, testing flow reduction when each is removed - const auto thread_budget = sensitivity_thread_budget(candidates.size()); + const auto thread_budget = thread_budget_from_env("NGRAPH_CORE_SENSITIVITY_THREADS", candidates.size()); if (thread_budget <= 1) { for (EdgeId eid : candidates) { auto maybe_result = evaluate_sensitivity_candidate( diff --git a/src/profiling.cpp b/src/profiling.cpp index cbbefa9..7acfed9 100644 --- a/src/profiling.cpp +++ b/src/profiling.cpp @@ -9,6 +9,7 @@ */ #include "netgraph/core/profiling.hpp" +#include // std::sort in dump() #include namespace netgraph::core { diff --git a/src/shortest_paths.cpp b/src/shortest_paths.cpp index b55231e..cd31d8c 100644 --- a/src/shortest_paths.cpp +++ b/src/shortest_paths.cpp @@ -1,10 +1,16 @@ /* Path enumeration from PredDAG (resolve_to_paths). */ #include "netgraph/core/shortest_paths.hpp" +#include "netgraph/core/constants.hpp" #include "netgraph/core/profiling.hpp" #include +#include +#include +#include #include -#include +#include +#include +#include #include #include @@ -30,6 +36,71 @@ static inline void group_parents(const PredDAG& dag, NodeId v, } } +std::pair, PredDAG> +path_to_pred_dag(const StrictMultiDiGraph& g, + std::span nodes, + std::span edges) { + const auto cost_view = g.cost_view(); + std::vector dist(static_cast(g.num_nodes()), + std::numeric_limits::max()); + PredDAG dag; + dag.parent_offsets.assign(static_cast(g.num_nodes() + 1), 0); + if (!nodes.empty()) { + dist[static_cast(nodes.front())] = 0; + for (std::size_t i = 1; i < nodes.size(); ++i) { + auto u = nodes[i - 1]; auto v = nodes[i]; auto e = edges[i - 1]; + dist[static_cast(v)] = + dist[static_cast(u)] + cost_view[static_cast(e)]; + dag.parent_offsets[static_cast(v + 1)] = 1; + } + for (std::size_t v = 1; v < dag.parent_offsets.size(); ++v) + dag.parent_offsets[v] += dag.parent_offsets[v - 1]; + dag.parents.resize(static_cast(dag.parent_offsets.back())); + dag.via_edges.resize(static_cast(dag.parent_offsets.back())); + for (std::size_t i = 1; i < nodes.size(); ++i) { + auto v = nodes[i]; + auto base = static_cast(dag.parent_offsets[static_cast(v)]); + dag.parents[base] = nodes[i - 1]; + dag.via_edges[base] = edges[i - 1]; + } + } + return {std::move(dist), std::move(dag)}; +} + +PredDAG make_path_dag(const StrictMultiDiGraph& g, std::span edges) { + if (edges.empty()) { + throw std::invalid_argument("make_path_dag: edges must be non-empty"); + } + const auto E = static_cast(g.num_edges()); + const auto esrc = g.edge_src_view(); + const auto edst = g.edge_dst_view(); + std::vector nodes; + nodes.reserve(edges.size() + 1); + std::vector seen(static_cast(g.num_nodes()), 0); + for (std::size_t i = 0; i < edges.size(); ++i) { + const auto e = edges[i]; + if (e < 0 || static_cast(e) >= E) { + throw std::invalid_argument("make_path_dag: edge id out of range"); + } + const auto u = esrc[static_cast(e)]; + const auto v = edst[static_cast(e)]; + if (i == 0) { + nodes.push_back(u); + seen[static_cast(u)] = 1; + } else if (u != nodes.back()) { + throw std::invalid_argument( + "make_path_dag: edges are not contiguous (edge source does not match the " + "previous edge's destination)"); + } + if (seen[static_cast(v)]) { + throw std::invalid_argument("make_path_dag: path revisits a node (must be a simple path)"); + } + seen[static_cast(v)] = 1; + nodes.push_back(v); + } + return path_to_pred_dag(g, nodes, edges).second; +} + std::vector>>> resolve_to_paths(const PredDAG& dag, NodeId src, NodeId dst, bool split_parallel_edges, @@ -49,9 +120,14 @@ resolve_to_paths(const PredDAG& dag, NodeId src, NodeId dst, struct Frame { NodeId node; std::size_t idx; std::vector>> groups; }; std::vector stack; stack.reserve(16); + // on_path[v] marks nodes on the current DFS stack. A well-formed PredDAG is + // acyclic, but zero-cost edges (or a caller-supplied DAG) can contain cycles; + // without this guard the enumeration below would walk them forever. + std::vector on_path(dag.parent_offsets.size() > 0 ? dag.parent_offsets.size() - 1 : 0, 0); // start from dst Frame start; start.node = dst; start.idx = 0; group_parents(dag, dst, start.groups); stack.push_back(std::move(start)); + on_path[static_cast(dst)] = 1; std::vector>> current; // reversed path accum @@ -59,11 +135,14 @@ resolve_to_paths(const PredDAG& dag, NodeId src, NodeId dst, auto& top = stack.back(); if (top.idx >= top.groups.size()) { // backtrack + on_path[static_cast(top.node)] = 0; stack.pop_back(); if (!current.empty()) current.pop_back(); continue; } auto [parent, edges] = top.groups[top.idx++]; + // Skip parents already on the current path (cycle in the input DAG). + if (parent != src && on_path[static_cast(parent)]) continue; current.emplace_back(top.node, std::move(edges)); if (parent == src) { // reached src; build forward segments: for each hop prev->next store (next, edges) @@ -141,6 +220,7 @@ resolve_to_paths(const PredDAG& dag, NodeId src, NodeId dst, current.pop_back(); continue; } + on_path[static_cast(parent)] = 1; stack.push_back(std::move(next)); } @@ -160,17 +240,6 @@ resolve_to_paths(const PredDAG& dag, NodeId src, NodeId dst, * Node-level tie-breaking for equal-cost nodes (prefers higher bottleneck capacity) - Early exit when specific destination is reached */ -#include "netgraph/core/shortest_paths.hpp" -#include -#include -#include -#include -#include -#include -#include -#include -#include "netgraph/core/constants.hpp" - namespace netgraph::core { namespace { @@ -205,11 +274,25 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, min_residual_to_node[static_cast(src)] = std::numeric_limits::max(); } - // pred_lists[v] stores predecessors for node v as (parent_node, [edges_from_parent]). - // In multipath mode, multiple parents with equal-cost paths are retained. - std::vector>>> pred_lists(static_cast(N)); + // Predecessor storage as a flat intrusive list: pred_head/pred_tail index into + // the ent_* arrays, whose entries are (parent, via_edge) pairs appended in + // discovery order. This avoids the per-node/per-group vector allocations that + // previously dominated SPF runtime (~70% of samples in malloc/free). + std::vector pred_head(static_cast(N), -1); + std::vector pred_tail(static_cast(N), -1); + std::vector ent_parent; ent_parent.reserve(static_cast(g.num_edges())); + std::vector ent_edge; ent_edge.reserve(static_cast(g.num_edges())); + std::vector ent_next; ent_next.reserve(static_cast(g.num_edges())); + auto pred_clear = [&](std::size_t v){ pred_head[v] = -1; pred_tail[v] = -1; }; + auto pred_append = [&](std::size_t v, NodeId p, EdgeId e){ + const auto idx = static_cast(ent_parent.size()); + ent_parent.push_back(p); ent_edge.push_back(e); ent_next.push_back(-1); + if (pred_head[v] < 0) { pred_head[v] = idx; } + else { ent_next[static_cast(pred_tail[v])] = idx; } + pred_tail[v] = idx; + }; if (src_allowed) { - pred_lists[static_cast(src)] = {}; + // source has no predecessors } else { // Source is out of range or masked out: no traversal, return empty DAG. PredDAG dag; @@ -234,6 +317,13 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, const bool require_cap = selection.require_capacity || has_residual; const bool multipath = multipath_arg; + // settled[v] is set once v is popped at its final distance. Equal-cost + // predecessor updates are only accepted while v is unsettled: with positive + // edge costs every equal-cost parent is discovered before v settles, and with + // zero-cost edges this guard is what keeps the PredDAG acyclic (previously a + // zero-cost pair u<->v recorded each node as the other's parent). + std::vector settled(static_cast(N), 0); + std::vector sel_buf; sel_buf.reserve(16); while (!pq.empty()) { // Extract min-cost node from priority queue. // Structured binding: auto [d_u, neg_res_u, u] = ... destructures the tuple. @@ -244,6 +334,7 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, // Skip residual-stale entries in single-path mode (same cost but outdated residual). if (!multipath && d_u == dist[static_cast(u)] && -neg_res_u < min_residual_to_node[static_cast(u)] - kEpsilon) continue; + settled[static_cast(u)] = 1; // Early exit optimization: record when we first reach destination. if (early_exit && u == dst_node && !have_best_dst) { best_dst_cost = d_u; have_best_dst = true; } @@ -266,7 +357,7 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, // Select best edge(s) from u to v according to policy. Cost min_edge_cost = std::numeric_limits::max(); - std::vector selected_edges; + std::vector& selected_edges = sel_buf; selected_edges.clear(); double best_rem_for_min_cost = -1.0; std::size_t j = i; int best_edge_id = -1; @@ -333,17 +424,18 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, // Relaxation: found shorter path to v, or equal-cost path with better capacity (single-path mode). if (new_cost < dist[v_idx] || - (!multipath && new_cost == dist[v_idx] && + (!multipath && new_cost == dist[v_idx] && !settled[v_idx] && path_residual > min_residual_to_node[v_idx] + kEpsilon)) { dist[v_idx] = new_cost; min_residual_to_node[v_idx] = path_residual; - pred_lists[v_idx].clear(); - pred_lists[v_idx].push_back({u, std::move(selected_edges)}); + pred_clear(v_idx); + for (auto sel_e : selected_edges) pred_append(v_idx, u, sel_e); pq.emplace(new_cost, -path_residual, v); // Negate residual for max-heap behavior } - // Multipath: found equal-cost alternative path to v. - else if (multipath && new_cost == dist[v_idx]) { - pred_lists[v_idx].push_back({u, std::move(selected_edges)}); + // Multipath: found equal-cost alternative path to v (only while v is + // unsettled; see the settled[] comment above). + else if (multipath && new_cost == dist[v_idx] && !settled[v_idx]) { + for (auto sel_e : selected_edges) pred_append(v_idx, u, sel_e); // Note: In multipath mode, we don't update min_residual_to_node because // we're collecting all equal-cost paths, not choosing based on residual. } @@ -353,7 +445,7 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, if (have_best_dst) { if (pq.empty() || std::get<0>(pq.top()) > best_dst_cost) break; } } - // Convert pred_lists to PredDAG using CSR-like layout. + // Convert flat predecessor lists to PredDAG using CSR-like layout. // parent_offsets[v]:parent_offsets[v+1] gives the range in parents/via_edges for node v. PredDAG dag; dag.parent_offsets.assign(static_cast(N+1), 0); @@ -361,7 +453,7 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, // Step 1: Count total predecessor entries per node. for (std::int32_t v=0; v(v)]) c += pe.second.size(); + for (std::int32_t i = pred_head[static_cast(v)]; i >= 0; i = ent_next[static_cast(i)]) ++c; dag.parent_offsets[static_cast(v+1)] = static_cast(c); } @@ -375,13 +467,10 @@ shortest_paths_core(const StrictMultiDiGraph& g, NodeId src, for (std::int32_t v=0; v(dag.parent_offsets[static_cast(v)]); std::size_t k = 0; - for (auto const& pe : pred_lists[static_cast(v)]) { - NodeId p = pe.first; - for (auto e : pe.second) { - dag.parents[base+k] = p; - dag.via_edges[base+k] = e; - ++k; - } + for (std::int32_t i = pred_head[static_cast(v)]; i >= 0; i = ent_next[static_cast(i)]) { + dag.parents[base+k] = ent_parent[static_cast(i)]; + dag.via_edges[base+k] = ent_edge[static_cast(i)]; + ++k; } } return {std::move(dist), std::move(dag)}; diff --git a/src/strict_multidigraph.cpp b/src/strict_multidigraph.cpp index 34dfc39..c582270 100644 --- a/src/strict_multidigraph.cpp +++ b/src/strict_multidigraph.cpp @@ -37,7 +37,14 @@ StrictMultiDiGraph StrictMultiDiGraph::from_arrays( throw std::invalid_argument("number of edges exceeds INT32_MAX"); } - // Invariants: ids within [0, num_nodes), non-negative weights + // Invariants: ids within [0, num_nodes), non-negative weights. + // SPF accumulates path costs as int64 without overflow checks and uses + // INT64_MAX as the unreachable sentinel, so the total of all edge costs + // (an upper bound on any simple path, and on one reverse-edge bounce) must + // stay below 2^62 or accumulated costs could wrap negative and silently + // corrupt results. + constexpr std::uint64_t kMaxTotalCost = (std::uint64_t{1} << 62); + std::uint64_t total_cost = 0; for (std::size_t i = 0; i < m; ++i) { if (src[i] < 0 || dst[i] < 0 || src[i] >= num_nodes || dst[i] >= num_nodes) { throw std::out_of_range("edge index out of range of num_nodes"); @@ -46,6 +53,13 @@ StrictMultiDiGraph StrictMultiDiGraph::from_arrays( throw std::invalid_argument("capacity must be >= 0"); } if (cost[i] < 0) { throw std::invalid_argument("cost must be >= 0"); } + const auto c = static_cast(cost[i]); + if (c >= kMaxTotalCost - total_cost) { + throw std::invalid_argument( + "total edge cost must stay below 2^62: larger accumulated path costs " + "overflow int64 cost arithmetic and silently corrupt results"); + } + total_cost += c; } // Gather initial arrays std::vector src_v(src.begin(), src.end()); diff --git a/tests/cpp/flow_policy_tests.cpp b/tests/cpp/flow_policy_tests.cpp index 5f51809..4b53ea2 100644 --- a/tests/cpp/flow_policy_tests.cpp +++ b/tests/cpp/flow_policy_tests.cpp @@ -6,6 +6,7 @@ #include "netgraph/core/algorithms.hpp" #include "netgraph/core/strict_multidigraph.hpp" #include "netgraph/core/types.hpp" +#include "test_utils.hpp" using namespace netgraph::core; @@ -349,3 +350,87 @@ TEST(FlowPolicyCore, EqualBalanced_ShortestPath_IgnoresHigherCostTier) { // Only shortest tier edges should carry flow expect_edge_flows_by_uv(fg, {{0,1,10.0}, {1,4,10.0}, {0,2,0.0}, {2,4,0.0}}); } + +// ============================================================================ +// Static paths (pinned path bundles) +// ============================================================================ + +// Each flow binds to ITS OWN bundle in supply order (the original Python +// semantics; the first C++ port bound every flow to bundle[0]). +TEST(FlowPolicyStatic, PerFlowBundleBinding) { + auto g = make_square1(); + FlowGraph fg(g); + auto be = make_cpu_backend(); auto algs = std::make_shared(be); auto gh = algs->build_graph(g); + ExecutionContext ctx(algs, gh); + FlowPolicy policy(ctx, FlowPolicyConfig{}); + + // Edge ids after (cost, src, dst) reordering match construction order here. + EdgeId short_path[2] = {0, 1}; // A->B->C, cost 2, cap 1 + EdgeId long_path[2] = {2, 3}; // A->D->C, cost 4, cap 2 + std::vector bundles; + bundles.push_back(make_path_dag(g, std::span(short_path, 2))); + bundles.push_back(make_path_dag(g, std::span(long_path, 2))); + policy.set_static_paths(0, 2, std::move(bundles)); + + auto [placed, left] = policy.place_demand(fg, 0, 2, 0, 3.0); + EXPECT_NEAR(placed, 3.0, 1e-9); + EXPECT_NEAR(left, 0.0, 1e-9); + ASSERT_EQ(policy.flow_count(), 2); + // Creation order == supply order; costs prove the binding. + std::vector> got; + for (auto const& kv : policy.flows()) got.emplace_back(kv.first.flowId, kv.second.cost); + std::sort(got.begin(), got.end()); + EXPECT_EQ(got[0].second, 2); + EXPECT_EQ(got[1].second, 4); +} + +// A bundle whose every walk is masked out is DOWN: no flow, no volume. +TEST(FlowPolicyStatic, MaskedBundleIsDown) { + using netgraph::core::test::make_bool_mask; + auto g = make_square1(); + FlowGraph fg(g); + auto be = make_cpu_backend(); auto algs = std::make_shared(be); auto gh = algs->build_graph(g); + ExecutionContext ctx(algs, gh); + + auto edge_mask = make_bool_mask(4); + edge_mask[0] = false; // A->B down -> the short bundle has no surviving walk + FlowPolicyConfig cfg; + cfg.edge_mask = std::span(edge_mask.get(), 4); + FlowPolicy policy(ctx, cfg); + + EdgeId short_path[2] = {0, 1}; + EdgeId long_path[2] = {2, 3}; + std::vector bundles; + bundles.push_back(make_path_dag(g, std::span(short_path, 2))); + bundles.push_back(make_path_dag(g, std::span(long_path, 2))); + policy.set_static_paths(0, 2, std::move(bundles)); + + auto [placed, left] = policy.place_demand(fg, 0, 2, 0, 3.0); + EXPECT_EQ(policy.flow_count(), 1); + EXPECT_NEAR(placed, 2.0, 1e-9); // only the long bundle carries + EXPECT_NEAR(left, 1.0, 1e-9); +} + +// Malformed bundles are rejected loudly; a foreign-graph DAG cannot slip through. +TEST(FlowPolicyStatic, ValidationRejectsForeignAndDisconnectedBundles) { + auto g = make_square1(); + auto be = make_cpu_backend(); auto algs = std::make_shared(be); auto gh = algs->build_graph(g); + ExecutionContext ctx(algs, gh); + FlowPolicy policy(ctx, FlowPolicyConfig{}); + + EdgeId short_path[2] = {0, 1}; + auto dag = make_path_dag(g, std::span(short_path, 2)); + + // Connects 0->2, not 1->3: structurally wrong for that demand. + { + std::vector b; b.push_back(dag); + EXPECT_THROW(policy.set_static_paths(1, 3, std::move(b)), std::invalid_argument); + } + // Shuffled via edge: entry no longer connects its parent to its node. + { + PredDAG bad = dag; + bad.via_edges[0] = 3; // D->C edge in place of a parent link + std::vector b; b.push_back(std::move(bad)); + EXPECT_THROW(policy.set_static_paths(0, 2, std::move(b)), std::invalid_argument); + } +} diff --git a/tests/cpp/k_shortest_paths_tests.cpp b/tests/cpp/k_shortest_paths_tests.cpp index 019b24e..30b1dc6 100644 --- a/tests/cpp/k_shortest_paths_tests.cpp +++ b/tests/cpp/k_shortest_paths_tests.cpp @@ -305,3 +305,27 @@ TEST(KShortestPaths, LargerOutOfOrderTopology) { } } } + +// Regression: a max_cost_factor below 1.0 puts the cost ceiling under the shortest +// path, so no path is admitted. The k > 1 loop opened with paths.back() on that empty +// vector -- undefined behaviour. The k == 1 early return masked it, so only k >= 2 hit +// it. All k must now agree and return no paths. +TEST(KSP, MaxCostFactorBelowOne_ReturnsEmptyForAllK) { + auto g = make_line_graph(3); + auto be = make_cpu_backend(); + Algorithms algs(be); + auto gh = algs.build_graph(g); + + for (int k : {1, 2, 5}) { + KspOptions opts; + opts.k = k; + opts.max_cost_factor = 0.5; + auto out = algs.ksp(gh, 0, 2, opts); + EXPECT_TRUE(out.empty()) << "k=" << k << " should admit no path under a sub-1.0 factor"; + } + // A factor of exactly 1.0 still admits the shortest path. + KspOptions ok; + ok.k = 3; + ok.max_cost_factor = 1.0; + EXPECT_EQ(algs.ksp(gh, 0, 2, ok).size(), 1u); +} diff --git a/tests/cpp/masking_tests.cpp b/tests/cpp/masking_tests.cpp index 7df9824..7af26d3 100644 --- a/tests/cpp/masking_tests.cpp +++ b/tests/cpp/masking_tests.cpp @@ -410,7 +410,7 @@ TEST(MaskingTests, KSPWithEdgeMask) { std::span(edge_mask.get(), g.num_edges())); // Should find remaining path(s) - EXPECT_GE(results.size(), 0); + EXPECT_EQ(results.size(), 1u); for (const auto& [dist, dag] : results) { if (dist[2] < std::numeric_limits::max()) { diff --git a/tests/cpp/max_flow_tests.cpp b/tests/cpp/max_flow_tests.cpp index 75f4cdc..cf14a92 100644 --- a/tests/cpp/max_flow_tests.cpp +++ b/tests/cpp/max_flow_tests.cpp @@ -1256,3 +1256,83 @@ TEST(MaxFlow, Sensitivity_ShortestPathVsMaxFlow) { } } } + +// ============================================================================ +// Regression: residual completion phase (flow cancellation) +// ============================================================================ + +// Successive shortest paths over forward-only SPF DAGs can strand capacity that +// only reverse-arc cancellation can recover. On this full-duplex topology the +// first tier routes 0->1->2->3 (cost 3), blocking both the direct 1->3 exit and +// the 2->3 exit; the true maximum needs to reroute it. Expected max flow is 3: +// 0->1->3 (1), 0->2->1->3 (1), 0->2->3 (1); the cut {0,2}|{1,3} has capacity 3. +TEST(MaxFlow, CompletionPhase_RecoversCancellation) { + std::vector src, dst; + std::vector cap; + std::vector cost; + auto add_duplex = [&](std::int32_t u, std::int32_t v, double c, std::int64_t k) { + src.push_back(u); dst.push_back(v); cap.push_back(c); cost.push_back(k); + src.push_back(v); dst.push_back(u); cap.push_back(c); cost.push_back(k); + }; + add_duplex(1, 3, 3.0, 17); + add_duplex(0, 2, 8.0, 17); + add_duplex(1, 2, 1.0, 1); + add_duplex(1, 0, 1.0, 1); + add_duplex(3, 2, 1.0, 1); + auto g = StrictMultiDiGraph::from_arrays(4, src, dst, cap, cost); + + auto be = make_cpu_backend(); + Algorithms algs(be); + auto gh = algs.build_graph(g); + + MaxFlowOptions opts; + opts.placement = FlowPlacement::Proportional; + opts.shortest_path = false; + + auto [total, summary] = algs.max_flow(gh, 0, 3, opts); + EXPECT_NEAR(total, 3.0, 1e-9) << "true max flow requires cancelling the first tier"; + + // Max-flow/min-cut duality: the reported cut capacity must equal the flow. + double cut_cap = 0.0; + auto capv = g.capacity_view(); + for (auto eid : summary.min_cut.edges) cut_cap += capv[static_cast(eid)]; + EXPECT_NEAR(cut_cap, total, 1e-9) << "min-cut capacity must equal total flow"; + + // Cost distribution must account for exactly the total flow. + double dist_sum = 0.0; + for (auto f : summary.flows) dist_sum += f; + EXPECT_NEAR(dist_sum, total, 1e-9); +} + +// The completion phase must respect masks: masking node 2 on the topology above +// limits the flow to the direct 0->1->3 route. +TEST(MaxFlow, CompletionPhase_RespectsNodeMask) { + std::vector src, dst; + std::vector cap; + std::vector cost; + auto add_duplex = [&](std::int32_t u, std::int32_t v, double c, std::int64_t k) { + src.push_back(u); dst.push_back(v); cap.push_back(c); cost.push_back(k); + src.push_back(v); dst.push_back(u); cap.push_back(c); cost.push_back(k); + }; + add_duplex(1, 3, 3.0, 17); + add_duplex(0, 2, 8.0, 17); + add_duplex(1, 2, 1.0, 1); + add_duplex(1, 0, 1.0, 1); + add_duplex(3, 2, 1.0, 1); + auto g = StrictMultiDiGraph::from_arrays(4, src, dst, cap, cost); + + auto be = make_cpu_backend(); + Algorithms algs(be); + auto gh = algs.build_graph(g); + + auto mask = make_bool_mask(4); + mask[2] = false; + + MaxFlowOptions opts; + opts.placement = FlowPlacement::Proportional; + opts.shortest_path = false; + opts.node_mask = std::span(mask.get(), 4); + + auto [total, summary] = algs.max_flow(gh, 0, 3, opts); + EXPECT_NEAR(total, 1.0, 1e-9) << "without node 2 only 0->1->3 remains (bottleneck 1)"; +} diff --git a/tests/cpp/shortest_paths_tests.cpp b/tests/cpp/shortest_paths_tests.cpp index 0080af2..261f964 100644 --- a/tests/cpp/shortest_paths_tests.cpp +++ b/tests/cpp/shortest_paths_tests.cpp @@ -298,3 +298,46 @@ TEST(ShortestPaths, RejectsMaskLengthMismatch) { opts2.edge_mask = std::span(edge_mask.get(), static_cast(g.num_edges() - 1)); EXPECT_THROW({ (void)algs.spf(gh, 0, opts2); }, std::invalid_argument); } + +// ============================================================================ +// Regression: zero-cost edges must not create cycles in the PredDAG +// ============================================================================ + +// A zero-cost pair 1<->2 previously recorded each node as the other's parent +// (both relaxations see an "equal cost" path), producing a cyclic PredDAG. +// EqualBalanced placement then stalled (returned 0 flow) and path enumeration +// walked the cycle forever. Equal-cost predecessors are now only accepted while +// the child is unsettled, which keeps the DAG acyclic by settle order. +TEST(ShortestPaths, ZeroCostEdges_PredDAGIsAcyclic) { + std::int32_t src_arr[4] = {0, 1, 2, 2}; + std::int32_t dst_arr[4] = {1, 2, 1, 3}; + double cap_arr[4] = {10.0, 10.0, 10.0, 10.0}; + std::int64_t cost_arr[4] = {1, 0, 0, 1}; + auto g = StrictMultiDiGraph::from_arrays(4, + std::span(src_arr, 4), std::span(dst_arr, 4), + std::span(cap_arr, 4), std::span(cost_arr, 4)); + + EdgeSelection sel; + sel.multi_edge = true; + auto [dist, dag] = shortest_paths(g, 0, std::nullopt, /*multipath=*/true, sel); + + EXPECT_EQ(dist[3], 2); + + // No 2-cycles: v's parent u must not also have v as a parent. + for (std::int32_t v = 0; v < 4; ++v) { + for (auto i = dag.parent_offsets[static_cast(v)]; + i < dag.parent_offsets[static_cast(v) + 1]; ++i) { + auto u = dag.parents[static_cast(i)]; + for (auto j = dag.parent_offsets[static_cast(u)]; + j < dag.parent_offsets[static_cast(u) + 1]; ++j) { + EXPECT_NE(dag.parents[static_cast(j)], v) + << "cycle " << u << "<->" << v << " in PredDAG"; + } + } + } + + // Path enumeration must terminate and yield the single simple path. + auto paths = resolve_to_paths(dag, 0, 3); + ASSERT_EQ(paths.size(), 1u); + EXPECT_EQ(paths[0].size(), 4u); // 0 -> 1 -> 2 -> 3 +} diff --git a/tests/cpp/test_utils.hpp b/tests/cpp/test_utils.hpp index 86a6b0e..f041799 100644 --- a/tests/cpp/test_utils.hpp +++ b/tests/cpp/test_utils.hpp @@ -5,7 +5,6 @@ #include #include "netgraph/core/strict_multidigraph.hpp" #include "netgraph/core/shortest_paths.hpp" -#include "netgraph/core/flow_graph.hpp" #include "netgraph/core/max_flow.hpp" namespace netgraph::core::test { @@ -436,38 +435,6 @@ inline void expect_pred_dag_semantically_valid(const StrictMultiDiGraph& g, } } -inline void expect_flow_conservation(const FlowGraph& fg, NodeId src, NodeId dst) { - const auto& g = fg.graph(); - auto row = g.row_offsets_view(); - auto col = g.col_indices_view(); - auto aei = g.adj_edge_index_view(); - auto in_row = g.in_row_offsets_view(); - auto in_col = g.in_col_indices_view(); - auto in_aei = g.in_adj_edge_index_view(); - auto flows = fg.edge_flow_view(); - - // For each intermediate node, inflow should equal outflow - for (std::int32_t u = 0; u < g.num_nodes(); ++u) { - if (u == src || u == dst) continue; - - double outflow = 0.0; - auto s = static_cast(row[static_cast(u)]); - auto e = static_cast(row[static_cast(u) + 1]); - for (std::size_t j = s; j < e; ++j) { - outflow += flows[static_cast(aei[j])]; - } - - double inflow = 0.0; - auto is = static_cast(in_row[static_cast(u)]); - auto ie = static_cast(in_row[static_cast(u) + 1]); - for (std::size_t j = is; j < ie; ++j) { - inflow += flows[static_cast(in_aei[j])]; - } - - EXPECT_NEAR(inflow, outflow, 1e-9) << "Flow not conserved at node " << u; - } -} - // FlowSummary validation helpers inline void validate_capacity_constraints(const StrictMultiDiGraph& g, const FlowSummary& summary) { ASSERT_EQ(summary.edge_flows.size(), static_cast(g.num_edges())) diff --git a/tests/py/conftest.py b/tests/py/conftest.py index 5e84fb3..ed33a8e 100644 --- a/tests/py/conftest.py +++ b/tests/py/conftest.py @@ -438,16 +438,6 @@ def _convert(g: ngc.StrictMultiDiGraph, dag: ngc.PredDAG): return _convert -@pytest.fixture -def make_pred_map(dag_to_pred_map): - """Alias fixture for converting PredDAG to {node: {parent: [EdgeId]}}. - - Provided to make intent clearer at call sites. - """ - - return dag_to_pred_map - - @pytest.fixture def assert_paths_concrete(): """Validate path tuples returned by resolve_to_paths. diff --git a/tests/py/test_flow_policy.py b/tests/py/test_flow_policy.py index 6bd2abc..774e903 100644 --- a/tests/py/test_flow_policy.py +++ b/tests/py/test_flow_policy.py @@ -7,7 +7,6 @@ from __future__ import annotations -import math from typing import Dict, Tuple import numpy as np @@ -90,10 +89,6 @@ def make_policy(config: str, algs: ngc.Algorithms, gh) -> ngc.FlowPolicy: } -def _almost_equal(a: float, b: float, tol: float = 1e-9) -> bool: - return math.isclose(a, b, rel_tol=0, abs_tol=tol) - - def _run_case( g: ngc.StrictMultiDiGraph, label: str, diff --git a/tests/py/test_flow_policy_validation.py b/tests/py/test_flow_policy_validation.py index c5e5ce4..9f2c62b 100644 --- a/tests/py/test_flow_policy_validation.py +++ b/tests/py/test_flow_policy_validation.py @@ -45,21 +45,6 @@ def parallel_paths_varying_capacities(): return _make_graph(num_nodes, src, dst, cap, cost) -@pytest.fixture -def parallel_paths_varying_costs(): - """3 parallel paths with different costs for cost-aware testing.""" - # S -> [M1, M2, M3] -> T - # Paths: S-M1-T (cost 10), S-M2-T (cost 20), S-M3-T (cost 30) - num_nodes = 5 - src = np.array([0, 0, 0, 1, 2, 3], dtype=np.int32) - dst = np.array([1, 2, 3, 4, 4, 4], dtype=np.int32) - cap = np.array([100.0, 100.0, 100.0, 100.0, 100.0, 100.0], dtype=np.float64) - # First hop costs: 10, 20, 30; Second hop costs: 10, 20, 30 - cost = np.array([10, 20, 30, 10, 20, 30], dtype=np.int64) - - return _make_graph(num_nodes, src, dst, cap, cost) - - # ============================================================================ # TEST 1: Path Distribution and Balance # ============================================================================ diff --git a/tests/py/test_graph_from_arrays.py b/tests/py/test_graph_from_arrays.py index 29e0316..19f8595 100644 --- a/tests/py/test_graph_from_arrays.py +++ b/tests/py/test_graph_from_arrays.py @@ -61,10 +61,9 @@ def test_from_arrays_self_loops_behavior(): dst = np.array([0], dtype=np.int32) # self-loop cap = np.array([1.0], dtype=np.float64) cost = np.array([1], dtype=np.int64) - # Either allowed or rejected; assert it doesn't crash and creates 1 edge or raises - try: - g = ngc.StrictMultiDiGraph.from_arrays(n, src, dst, cap, cost, _make_ext_ids(1)) - assert g.num_edges() >= 1 - except Exception: - # Accept rejection behavior too - pass + # Self-loops are accepted: StrictMultiDiGraph is a multigraph and from_arrays + # only rejects out-of-range ids and negative capacity/cost. + g = ngc.StrictMultiDiGraph.from_arrays(n, src, dst, cap, cost, _make_ext_ids(1)) + assert g.num_edges() == 1 + assert g.edge_src_view()[0] == 0 + assert g.edge_dst_view()[0] == 0 diff --git a/tests/py/test_policy_vs_maxflow_equivalence.py b/tests/py/test_policy_vs_maxflow_equivalence.py index 0d51d9a..cf5815a 100644 --- a/tests/py/test_policy_vs_maxflow_equivalence.py +++ b/tests/py/test_policy_vs_maxflow_equivalence.py @@ -60,25 +60,6 @@ def _policy_for( return ngc.FlowPolicy(algs, gh, cfg) -def _max_flow_for( - algs: ngc.Algorithms, - gh, - placement: ngc.FlowPlacement, - shortest_path: bool, - require_capacity: bool, -) -> float: - total, _ = algs.max_flow( - gh, - 0, - 1, # src, dst are supplied by fixtures through to_handle(graph) below; we override per case - flow_placement=placement, - shortest_path=shortest_path, - require_capacity=require_capacity, - with_edge_flows=False, - ) - return float(total) - - @pytest.mark.parametrize( "graph_label", [ diff --git a/tests/py/test_review_regressions.py b/tests/py/test_review_regressions.py new file mode 100644 index 0000000..ea83104 --- /dev/null +++ b/tests/py/test_review_regressions.py @@ -0,0 +1,499 @@ +"""Regression tests for the 2026-08 code review findings. + +Each test locks in the fixed behavior for a confirmed defect: + +1. max_flow under-reported when the SSP tier loop needed flow cancellation; + its own min_cut then contradicted total_flow (duality violation). +2. Zero-cost edges produced a cyclic PredDAG: EqualBalanced placement returned + 0 flow and resolve_to_paths never terminated. +3. k_shortest_paths enumerated every equal-cost spur path (exponential in ECMP + width; a 22-stage ladder needed ~19 s and ~3.8 GB for k=3). +4. batch_max_flow results must be identical to per-pair calls (now computed in + parallel). +5. FlowPolicy.place_demand silently routed a second (src, dst) pair over the + first pair's paths. +6. StrictMultiDiGraph accepted cost totals that overflow int64 path arithmetic. +7. FlowState.compute_min_cut treated pre-existing usage from a custom + residual_init as cancellable flow. +8. batch_max_flow rejected a genuine int32 `pairs` array on Windows, and read + non-contiguous input as if it were packed. +9-10. FlowPolicy accepted a FlowGraph built from a different graph, and + rebalance_demand bypassed the (src, dst) guard. +11-12. batch_max_flow and ksp validated inputs later (or not at all) compared + with their single-pair counterparts. +13. _docs.py declared types that did not match runtime. +14. The same class of user error raised different exception types. +15. k_shortest_paths read paths.back() on an empty vector when + max_cost_factor < 1.0 and k > 1. +""" + +from __future__ import annotations + +import numpy as np +import pytest + +import netgraph_core as ngc + + +def _graph(num_nodes, edges): + """edges: list of (src, dst, capacity, cost).""" + s, d, c, k = zip(*edges, strict=False) + return ngc.StrictMultiDiGraph.from_arrays( + num_nodes, + np.array(s, dtype=np.int32), + np.array(d, dtype=np.int32), + np.array(c, dtype=np.float64), + np.array(k, dtype=np.int64), + np.arange(len(s), dtype=np.int64), + ) + + +def _bidir(links): + out = [] + for u, v, cap, cost in links: + out.append((u, v, cap, cost)) + out.append((v, u, cap, cost)) + return out + + +@pytest.fixture +def algs(): + return ngc.Algorithms(ngc.Backend.cpu()) + + +class TestMaxFlowCompletion: + """Finding 1: forward-only SSP stranded capacity that needs cancellation.""" + + def _cancellation_graph(self): + # s=0, a=1, b=2, t=3; all links full duplex. True max flow is 3.0: + # 0->1->3 (1), 0->2->1->3 (1), 0->2->3 (1); cut {0,2}|{1,3} has cap 3. + return _graph( + 4, + _bidir( + [ + (1, 3, 3.0, 17), + (0, 2, 8.0, 17), + (1, 2, 1.0, 1), + (1, 0, 1.0, 1), + (3, 2, 1.0, 1), + ] + ), + ) + + def test_exact_max_flow(self, algs): + g = self._cancellation_graph() + pg = algs.build_graph(g) + total, _ = algs.max_flow(pg, 0, 3) + assert total == pytest.approx(3.0) + + def test_min_cut_duality(self, algs): + g = self._cancellation_graph() + pg = algs.build_graph(g) + total, summary = algs.max_flow(pg, 0, 3) + cap = np.asarray(g.capacity_view()) + cut = np.asarray(summary.min_cut.edges) + assert cut.size > 0 + assert float(cap[cut].sum()) == pytest.approx(total) + + def test_cost_distribution_accounts_for_total(self, algs): + g = self._cancellation_graph() + pg = algs.build_graph(g) + total, summary = algs.max_flow(pg, 0, 3) + assert float(np.asarray(summary.flows).sum()) == pytest.approx(total) + + def test_completion_respects_masks(self, algs): + g = self._cancellation_graph() + pg = algs.build_graph(g) + mask = np.ones(4, dtype=bool) + mask[2] = False # only 0->1->3 remains; bottleneck is cap(0->1) = 1 + total, _ = algs.max_flow(pg, 0, 3, node_mask=mask) + assert total == pytest.approx(1.0) + + +class TestZeroCostEdges: + """Finding 2: zero-cost edges made the PredDAG cyclic.""" + + def _zero_cost_graph(self): + # 0 ->(1) 1 <->(0) 2 ->(1) 3 + return _graph( + 4, + [(0, 1, 10.0, 1), (1, 2, 10.0, 0), (2, 1, 10.0, 0), (2, 3, 10.0, 1)], + ) + + def test_pred_dag_acyclic(self, algs): + g = self._zero_cost_graph() + pg = algs.build_graph(g) + _, dag = algs.spf(pg, 0, None, multipath=True) + po = np.asarray(dag.parent_offsets) + pa = np.asarray(dag.parents) + for v in range(4): + for i in range(po[v], po[v + 1]): + u = pa[i] + assert v not in pa[po[u] : po[u + 1]], f"cycle {u}<->{v}" + + def test_equal_balanced_places_flow(self, algs): + g = self._zero_cost_graph() + pg = algs.build_graph(g) + total, _ = algs.max_flow( + pg, 0, 3, flow_placement=ngc.FlowPlacement.EQUAL_BALANCED + ) + assert total == pytest.approx(10.0) + + def test_resolve_to_paths_terminates(self, algs): + g = self._zero_cost_graph() + pg = algs.build_graph(g) + _, dag = algs.spf(pg, 0, None, multipath=True) + paths = dag.resolve_to_paths(0, 3) # previously never returned + assert [n for n, _ in paths[0]] == [0, 1, 2, 3] + + +class TestKspEcmpLadder: + """Finding 3: spur enumeration was exponential in equal-cost path count.""" + + def test_ladder_is_tractable(self, algs): + # 30 stages of parallel 2-hop diamonds: 2^30 equal-cost paths. + # Pre-fix this was unreachable (22 stages already took ~19 s / 3.8 GB). + stages = 30 + edges = [] + nid = 0 + + def new(): + nonlocal nid + nid += 1 + return nid - 1 + + main = [new()] + for _ in range(stages): + nxt, a, b = new(), new(), new() + for u, v in ((main[-1], a), (a, nxt), (main[-1], b), (b, nxt)): + edges.append((u, v, 10.0, 1)) + main.append(nxt) + g = _graph(nid, edges) + pg = algs.build_graph(g) + out = algs.ksp(pg, main[0], main[-1], k=4) + assert len(out) == 4 + # All returned paths are shortest (equal cost) and distinct. + costs = [dist[main[-1]] for dist, _ in out] + assert costs == [2 * stages] * 4 + sigs = {tuple(dag.via_edges) for _, dag in out} + assert len(sigs) == 4 + + +class TestBatchMaxFlowParity: + """Finding 4: batch results must equal per-pair results (now parallel).""" + + def test_batch_equals_serial(self, algs): + rng = np.random.default_rng(7) + n = 12 + edges = [] + for _ in range(40): + u, v = rng.integers(0, n, size=2) + if u != v: + edges.append((int(u), int(v), float(rng.integers(1, 8)), 1)) + g = _graph(n, edges) + pg = algs.build_graph(g) + pairs = np.array([[i, (i + 5) % n] for i in range(8)], dtype=np.int32) + batch = algs.batch_max_flow(pg, pairs, with_edge_flows=True) + for i, (a, b) in enumerate(pairs): + total, summary = algs.max_flow(pg, int(a), int(b), with_edge_flows=True) + assert batch[i].total_flow == pytest.approx(total) + np.testing.assert_array_equal( + np.asarray(batch[i].edge_flows), np.asarray(summary.edge_flows) + ) + + +class TestFlowPolicySrcDstGuard: + """Finding 5: reusing a policy for a different pair corrupted placement.""" + + def test_mismatched_pair_raises(self, algs): + g = _graph(4, [(0, 1, 10.0, 1), (2, 3, 10.0, 1)]) + pg = algs.build_graph(g) + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + placed, _ = policy.place_demand(fg, 0, 1, 0, 5.0) + assert placed == pytest.approx(5.0) + with pytest.raises(ValueError, match="different \\(src, dst\\)"): + policy.place_demand(fg, 2, 3, 0, 5.0) + + def test_same_pair_and_after_remove_ok(self, algs): + g = _graph(4, [(0, 1, 10.0, 1), (2, 3, 10.0, 1)]) + pg = algs.build_graph(g) + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + assert policy.place_demand(fg, 0, 1, 0, 5.0)[0] == pytest.approx(5.0) + assert policy.place_demand(fg, 0, 1, 0, 2.0)[0] == pytest.approx(2.0) + policy.remove_demand(fg) + assert policy.place_demand(fg, 2, 3, 0, 5.0)[0] == pytest.approx(5.0) + np.testing.assert_allclose(np.asarray(fg.edge_flow_view()), [0.0, 5.0]) + + +class TestCostOverflowGuard: + """Finding 6: cost totals near INT64_MAX silently wrapped in SPF.""" + + def test_total_at_2_pow_62_rejected(self): + big = 1 << 62 + with pytest.raises(ValueError, match="2\\^62"): + _graph(3, [(0, 1, 10.0, big), (1, 2, 10.0, big)]) + + def test_large_but_safe_costs_accepted(self, algs): + big = 1 << 60 + g = _graph(3, [(0, 1, 10.0, big), (1, 2, 10.0, big)]) + pg = algs.build_graph(g) + dist, _ = algs.spf(pg, 0, None, dtype="int64") + assert dist[2] == 2 * big # no wraparound + + +class TestFlowStateCustomResidual: + """Finding 7: custom residual_init desynced min-cut's notion of flow.""" + + def test_min_cut_uses_own_placed_flow(self): + g = _graph(3, [(0, 1, 10.0, 1), (1, 2, 10.0, 1)]) + fs = ngc.FlowState(g, np.array([4.0, 4.0])) + placed = fs.place_max_flow(0, 2, ngc.FlowPlacement.PROPORTIONAL) + assert placed == pytest.approx(4.0) + # Reverse arcs must be keyed on this state's own edge_flow (4.0), not + # capacity - residual (10.0, which counts pre-existing usage). + np.testing.assert_allclose(np.asarray(fs.edge_flow_view()), [4.0, 4.0]) + cut = np.asarray(fs.compute_min_cut(0).edges) + assert cut.size > 0 + + +class TestBatchPairsDtype: + """Finding 8: `batch_max_flow` rejected a genuine int32 array on Windows. + + The dtype check compared buffer format strings. NumPy spells int32 as + NPY_LONG ('l') on LLP64 (Windows) but NPY_INT ('i') on LP64, while + pybind11's `format_descriptor` is always 'i', so the check threw on + Windows for a correctly-typed array. Parametrizing over both 32-bit + spellings exercises whichever one is the platform's alias for int32. + """ + + def _line_graph(self): + return _graph(3, [(0, 1, 5.0, 1), (1, 2, 5.0, 1)]) + + @pytest.mark.parametrize("dtype", [np.int32, np.intc]) + def test_accepts_every_int32_spelling(self, algs, dtype): + g = self._line_graph() + pg = algs.build_graph(g) + pairs = np.array([[0, 2]], dtype=dtype) + assert np.dtype(dtype).itemsize == 4 and np.dtype(dtype).kind == "i" + out = algs.batch_max_flow(pg, pairs) + assert out[0].total_flow == pytest.approx(5.0) + + def test_rejects_wrong_dtype(self, algs): + pg = algs.build_graph(self._line_graph()) + with pytest.raises(TypeError, match="int32"): + algs.batch_max_flow(pg, np.array([[0, 2]], dtype=np.int64)) + + def test_rejects_non_contiguous(self, algs): + # A strided view would otherwise be read as if it were packed, silently + # turning [[0, 2]] into the pair (0, 99). + pg = algs.build_graph(self._line_graph()) + strided = np.array([[0, 99, 2, 99]], dtype=np.int32)[:, ::2] + assert not strided.flags["C_CONTIGUOUS"] + with pytest.raises(TypeError, match="C-contiguous"): + algs.batch_max_flow(pg, strided) + + +class TestFlowPolicyTargetGuards: + """Findings 9-10: FlowPolicy accepted uses that silently produced wrong answers.""" + + def _two_graphs_same_edge_count(self): + # Same edge count, different topology: the residual-length check that caught + # the mismatched-graph case by accident does not fire here. + a = _graph(4, [(0, 1, 5.0, 1), (1, 3, 5.0, 1), (0, 2, 1.0, 1)]) + b = _graph(4, [(0, 2, 9.0, 1), (2, 3, 9.0, 1), (0, 1, 1.0, 1)]) + return a, b + + def test_flowgraph_from_a_different_graph_is_rejected(self, algs): + ga, gb = self._two_graphs_same_edge_count() + policy = ngc.FlowPolicy(algs, algs.build_graph(ga), ngc.FlowPolicyConfig()) + with pytest.raises(ValueError, match="different StrictMultiDiGraph"): + policy.place_demand(ngc.FlowGraph(gb), 0, 3, 0, 100.0) + + def test_rebalance_demand_rejects_a_different_pair(self, algs): + # remove_demand() empties flows_, so rebalance_demand must check the pair + # itself rather than relying on the check inside place_demand. + g = _graph(4, [(0, 1, 10.0, 1), (2, 3, 10.0, 1)]) + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, algs.build_graph(g), ngc.FlowPolicyConfig()) + policy.place_demand(fg, 0, 1, 0, 5.0) + with pytest.raises(ValueError, match="different \\(src, dst\\)"): + policy.rebalance_demand(fg, 2, 3, 0, 5.0) + # The matching pair still rebalances, and retargeting after remove_demand works. + policy.rebalance_demand(fg, 0, 1, 0, 2.0) + policy.remove_demand(fg) + assert policy.place_demand(fg, 2, 3, 0, 5.0)[0] == pytest.approx(5.0) + + +class TestBindingValidationConsistency: + """Findings 11-12: validation that differed between analogous entry points.""" + + def _line(self): + return _graph(3, [(0, 1, 5.0, 1), (1, 2, 5.0, 1)]) + + def test_batch_max_flow_rejects_out_of_range_like_max_flow(self, algs): + pg = algs.build_graph(self._line()) + # Previously this returned total_flow=0.0 and the bad id vanished into the batch. + with pytest.raises(ValueError, match="out of range"): + algs.batch_max_flow(pg, np.array([[0, 99]], dtype=np.int32)) + with pytest.raises(ValueError, match="out of range"): + algs.max_flow(pg, 0, 99) + + def test_ksp_validates_dtype_even_when_no_paths_exist(self, algs): + pg = algs.build_graph(self._line()) + # 2 -> 0 is unreachable, so the results loop never runs; the dtype check used + # to live inside that loop and silently accepted the bad value. + with pytest.raises(ValueError, match="dtype must be"): + algs.ksp(pg, 2, 0, k=1, dtype="float32") + + def test_algorithms_backend_argument_is_nameable(self): + assert isinstance(ngc.Algorithms(backend=ngc.Backend.cpu()), ngc.Algorithms) + + def test_unreachable_flow_record_class_is_not_exported(self): + import _netgraph_core + + # Nothing could construct or receive it, and the name collided with the C++ + # alias `using Flow = double`. + assert not hasattr(_netgraph_core, "Flow") + + +class TestTypeStubAccuracy: + """Finding 13: _docs.py declared types that did not match runtime.""" + + def test_min_cut_edges_is_an_int32_array_not_a_list(self, algs): + g = _graph(3, [(0, 1, 5.0, 1), (1, 2, 5.0, 1)]) + _, summary = algs.max_flow(algs.build_graph(g), 0, 2) + edges = summary.min_cut.edges + assert isinstance(edges, np.ndarray) + assert edges.dtype == np.int32 + + def test_stub_declares_only_names_that_exist(self): + import _netgraph_core + + from netgraph_core import _docs + + declared = { + n + for n in dir(_docs) + if not n.startswith("_") and isinstance(getattr(_docs, n), type) + } + missing = {n for n in declared if not hasattr(_netgraph_core, n)} + assert not missing, f"_docs.py declares nonexistent runtime types: {missing}" + + +class TestErrorTypeConsistency: + """Finding 14: the same class of user error raised different exception types.""" + + @pytest.fixture + def pg(self, algs): + return algs.build_graph(_graph(3, [(0, 1, 5.0, 1), (1, 2, 5.0, 1)])) + + def test_spf_residual_length_raises_type_error_like_masks(self, algs, pg): + # Previously the residual length check fell through to the C++ core, which + # throws std::invalid_argument -> ValueError, while every mask length check + # in the binding layer raises TypeError. + with pytest.raises(TypeError, match="residual length must equal"): + algs.spf(pg, 0, residual=np.zeros(5)) + with pytest.raises(TypeError, match="node_mask length must equal"): + algs.spf(pg, 0, node_mask=np.ones(7, dtype=bool)) + + def test_wrong_typed_arguments_raise_type_error_not_runtime_error(self, algs, pg): + # These hand-rolled casts used to surface as + # "RuntimeError: Unable to cast ... to C++ type '?'", which no `except + # TypeError` catches and which names nothing actionable. + with pytest.raises(TypeError, match="must be a StrictMultiDiGraph"): + algs.build_graph(5) + with pytest.raises(TypeError, match="must be an Algorithms"): + ngc.FlowPolicy("not-algorithms", pg, ngc.FlowPolicyConfig()) + + +class TestKspCostCeiling: + """Finding 15: k>1 with max_cost_factor < 1.0 read paths.back() on an empty vector. + + The ceiling lands below the shortest path, so nothing is admitted. The k == 1 + branch returned early and masked the undefined behaviour; k >= 2 fell into the + spur loop, which opens with `paths.back()`. + """ + + @pytest.mark.parametrize("k", [1, 2, 5]) + @pytest.mark.parametrize("factor", [0.5, 0.99]) + def test_sub_unit_factor_returns_empty_for_every_k(self, algs, k, factor): + g = _graph(3, [(0, 1, 5.0, 1), (1, 2, 5.0, 1)]) + pg = algs.build_graph(g) + assert algs.ksp(pg, 0, 2, k=k, max_cost_factor=factor) == [] + + @pytest.mark.parametrize("k", [1, 2, 5]) + def test_unit_factor_still_admits_the_shortest_path(self, algs, k): + g = _graph(3, [(0, 1, 5.0, 1), (1, 2, 5.0, 1)]) + pg = algs.build_graph(g) + assert len(algs.ksp(pg, 0, 2, k=k, max_cost_factor=1.0)) == 1 + + +class TestStubConstructorConsistency: + """Finding 16: widened stubs declared no __init__ for classes that need one. + + `_docs.py` is wired to type checkers, so a stub whose constructor disagrees + with the binding turns "unchecked" into "confidently wrong": pyright rejected + every real `FlowIndex(src, dst, cls, id)` call in the downstream consumer. + This repo's own pyright missed it because `tests/**` is excluded, so assert + the correspondence at runtime instead. + """ + + def _runtime_classes(self): + import _netgraph_core + + for name in sorted(n for n in dir(_netgraph_core) if not n.startswith("_")): + obj = getattr(_netgraph_core, name) + if isinstance(obj, type): + yield name, obj + + def test_stub_declares_init_wherever_construction_needs_arguments(self): + from netgraph_core import _docs + + missing = [] + for name, rt in self._runtime_classes(): + stub = getattr(_docs, name, None) + if stub is None: + continue # not part of the typed surface + try: + rt() + continue # zero-arg construction works; a bare stub is fine + except Exception: + pass + doc = rt.__init__.__doc__ or "" + if "__init__" not in doc: + continue # no bound constructor at all (e.g. result-only types) + if "__init__" not in vars(stub): + missing.append(name) + assert not missing, ( + f"stub classes need an __init__ matching the binding: {missing}" + ) + + def test_stub_attributes_exist_at_runtime(self): + from netgraph_core import _docs + + wrong = [] + for name, rt in self._runtime_classes(): + stub = getattr(_docs, name, None) + if stub is None: + continue + declared = { + a + for a in vars(stub) + if not a.startswith("_") and not callable(vars(stub)[a]) + } + declared |= set(getattr(stub, "__annotations__", {})) + for attr in declared: + if not hasattr(rt, attr): + wrong.append(f"{name}.{attr}") + assert not wrong, f"_docs.py declares attributes absent at runtime: {wrong}" + + def test_flow_index_constructor_matches_the_binding(self): + # The exact call shape the downstream consumer uses. + idx = ngc.FlowIndex(1, 2, 3, 4) + assert (idx.src, idx.dst, idx.flowClass, idx.flowId) == (1, 2, 3, 4) + with pytest.raises(TypeError): + ngc.FlowIndex(src=1, dst=2, flowClass=3, flowId=4) # positional-only + with pytest.raises(AttributeError): + idx.src = 9 # read-only, as the stub's properties declare diff --git a/tests/py/test_static_paths.py b/tests/py/test_static_paths.py new file mode 100644 index 0000000..eb94689 --- /dev/null +++ b/tests/py/test_static_paths.py @@ -0,0 +1,392 @@ +"""FlowPolicy.set_static_paths: pinned path bundles (MPLS-style routing). + +Semantics under test (design-panel reviewed): +- One flow per usable bundle, each permanently bound to its own bundle in + supply order (the original Python semantics; the first C++ port bound every + flow to bundle[0]). +- Bundles are validated against the graph and pruned against the policy's + masks; a bundle with no surviving src->dst walk is DOWN and creates no flow. +- Flow cost is the min-cost src->dst walk of the PRUNED bundle. +- Pinned policies never grow their flow set and never reoptimize; + max_path_cost/-factor, min_flow_count and reoptimize_flows_on_each_placement + are inert. +- EqualBalanced spreads over the usable (up) bundles only, and the rebalance + loop is iterative (many heterogeneous pinned bundles must not blow the stack). +- PredDAG.from_edges builds a single-path bundle from a contiguous edge list. +""" + +from __future__ import annotations + +import numpy as np +import pytest + +import netgraph_core as ngc + + +def _graph(num_nodes, edges): + """edges: list of (src, dst, capacity, cost).""" + s, d, c, k = zip(*edges, strict=False) + return ngc.StrictMultiDiGraph.from_arrays( + num_nodes, + np.array(s, dtype=np.int32), + np.array(d, dtype=np.int32), + np.array(c, dtype=np.float64), + np.array(k, dtype=np.int64), + np.arange(len(s), dtype=np.int64), + ) + + +def _eid(g, u, v): + """Edge id of u->v after the graph's (cost, src, dst) reordering.""" + es, ed = g.edge_src_view(), g.edge_dst_view() + (idx,) = np.where((es == u) & (ed == v)) + assert idx.size == 1, f"expected exactly one {u}->{v} edge" + return int(idx[0]) + + +@pytest.fixture +def algs(): + return ngc.Algorithms(ngc.Backend.cpu()) + + +@pytest.fixture +def square(algs): + """0->1->2 (cost 1+1, cap 1 each) and 0->3->2 (cost 2+2, cap 2 each).""" + g = _graph(4, [(0, 1, 1.0, 1), (1, 2, 1.0, 1), (0, 3, 2.0, 2), (3, 2, 2.0, 2)]) + pg = algs.build_graph(g) + short = ngc.PredDAG.from_edges(g, [_eid(g, 0, 1), _eid(g, 1, 2)]) + long = ngc.PredDAG.from_edges(g, [_eid(g, 0, 3), _eid(g, 3, 2)]) + return g, pg, short, long + + +class TestFromEdges: + def test_builds_usable_single_path_dag(self, algs, square): + g, pg, short, _ = square + fs = ngc.FlowState(g) + placed = fs.place_on_dag( + 0, 2, short, float("inf"), ngc.FlowPlacement.PROPORTIONAL + ) + assert placed == pytest.approx(1.0) # bottleneck of the 0->1->2 path + + def test_rejects_empty_out_of_range_non_contiguous_and_revisits(self, algs): + g = _graph(3, [(0, 1, 1.0, 1), (1, 2, 1.0, 1), (2, 0, 1.0, 1)]) + e01, e12, e20 = _eid(g, 0, 1), _eid(g, 1, 2), _eid(g, 2, 0) + with pytest.raises(ValueError, match="non-empty"): + ngc.PredDAG.from_edges(g, []) + with pytest.raises(ValueError, match="out of range"): + ngc.PredDAG.from_edges(g, [99]) + with pytest.raises(ValueError, match="not contiguous"): + ngc.PredDAG.from_edges(g, [e01, e20]) + with pytest.raises(ValueError, match="simple path"): + ngc.PredDAG.from_edges(g, [e01, e12, e20]) # returns to node 0 + + +class TestPerFlowBinding: + """The port bug this redesign fixes: every flow bound to bundle[0].""" + + def test_each_flow_bound_to_its_own_bundle_in_supply_order(self, algs, square): + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + policy.set_static_paths(0, 2, [short, long]) + placed, left = policy.place_demand(fg, 0, 2, 0, 3.0) + assert placed == pytest.approx(3.0) # 1 on the short path + 2 on the long + assert left == pytest.approx(0.0) + # Ordinal binding: creation order == supply order; costs prove which + # bundle each flow holds (short = 2, long = 4). + by_creation = sorted(policy.flows.items(), key=lambda kv: kv[0][3]) + assert [v[2] for _, v in by_creation] == [2, 4] + assert [v[3] for _, v in by_creation] == pytest.approx([1.0, 2.0]) + + def test_supply_order_is_placement_precedence(self, algs, square): + """Proportional pinned placement is greedy in bundle-supply order.""" + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + policy.set_static_paths(0, 2, [long, short]) # long first this time + placed, _ = policy.place_demand(fg, 0, 2, 0, 2.0) + assert placed == pytest.approx(2.0) + by_creation = sorted(policy.flows.items(), key=lambda kv: kv[0][3]) + # First-supplied bundle (long, cost 4) absorbed everything. + assert [v[2] for _, v in by_creation] == [4, 2] + assert [v[3] for _, v in by_creation] == pytest.approx([2.0, 0.0]) + + +class TestEqualBalanced: + def test_eb_equalizes_to_bottleneck_bundle(self, algs, square): + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy( + algs, + pg, + ngc.FlowPolicyConfig(flow_placement=ngc.FlowPlacement.EQUAL_BALANCED), + ) + policy.set_static_paths(0, 2, [short, long]) + placed, _ = policy.place_demand(fg, 0, 2, 0, 3.0) + # ECMP over 2 LSPs; the short path caps each at ~1.0. + assert placed == pytest.approx(2.0, abs=1e-3) + vols = [v[3] for v in policy.flows.values()] + assert max(vols) - min(vols) < 1e-3 + + def test_eb_spreads_over_up_bundles_only(self, algs, square): + g, pg, short, long = square + em = np.ones(4, dtype=bool) + em[_eid(g, 0, 1)] = False # short path down + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy( + algs, + pg, + ngc.FlowPolicyConfig(flow_placement=ngc.FlowPlacement.EQUAL_BALANCED), + edge_mask=em, + ) + policy.set_static_paths(0, 2, [short, long]) + placed, _ = policy.place_demand(fg, 0, 2, 0, 3.0) + # One up LSP; head-end ECMP over up tunnels only -> full 2.0 on the long + # path, not volume/2 as a supplied-count divisor would give. + assert policy.flow_count() == 1 + assert placed == pytest.approx(2.0, abs=1e-3) + + def test_many_heterogeneous_bundles_do_not_blow_the_stack(self, algs): + """256 pinned parallel LSPs with one small bundle: the EB rebalance loop + (previously recursion of depth ~U * ln(imbalance/kMinFlow)) must survive.""" + n_paths = 256 + edges = [] + # src=0, dst=1, path i via node 2+i with capacity 100 except one runt. + for i in range(n_paths): + mid = 2 + i + cap = 0.5 if i == 0 else 100.0 + edges.append((0, mid, cap, 1)) + edges.append((mid, 1, cap, 1)) + g = _graph(2 + n_paths, edges) + pg = algs.build_graph(g) + bundles = [ + ngc.PredDAG.from_edges(g, [_eid(g, 0, 2 + i), _eid(g, 2 + i, 1)]) + for i in range(n_paths) + ] + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy( + algs, + pg, + ngc.FlowPolicyConfig(flow_placement=ngc.FlowPlacement.EQUAL_BALANCED), + ) + policy.set_static_paths(0, 1, bundles) + placed, _ = policy.place_demand(fg, 0, 1, 0, 10_000.0) + # ECMP: the runt bundle (cap 0.5) bounds every LSP. + assert placed == pytest.approx(n_paths * 0.5, rel=1e-2) + + def test_second_placement_is_not_incremental(self, algs, square): + """Documented: per-call volume defines the EB per-flow target, so a second + smaller placement adds nothing (parity with dynamic EB + max_flow_count).""" + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy( + algs, + pg, + ngc.FlowPolicyConfig(flow_placement=ngc.FlowPlacement.EQUAL_BALANCED), + ) + policy.set_static_paths(0, 2, [short, long]) + first, _ = policy.place_demand(fg, 0, 2, 0, 2.0) + assert first == pytest.approx(2.0, abs=1e-3) + # The smaller per-call volume yields a smaller per-flow target that the + # flows already meet, so nothing new is placed and the new volume is + # returned as leftover. + second, second_left = policy.place_demand(fg, 0, 2, 0, 1.0) + assert second == pytest.approx(0.0, abs=1e-3) + assert second_left == pytest.approx(1.0, abs=1e-3) + assert policy.placed_demand() == pytest.approx(first, abs=1e-3) + + +class TestDiminishingReturnsArming: + def test_more_bundles_than_window_all_receive_volume(self, algs): + """16 LSPs x cap 1 against volume 10000: the diminishing-returns window + (8) must not fire before every pinned flow was visited once.""" + n_paths = 16 + edges = [] + for i in range(n_paths): + mid = 2 + i + edges.append((0, mid, 1.0, 1)) + edges.append((mid, 1, 1.0, 1)) + g = _graph(2 + n_paths, edges) + pg = algs.build_graph(g) + bundles = [ + ngc.PredDAG.from_edges(g, [_eid(g, 0, 2 + i), _eid(g, 2 + i, 1)]) + for i in range(n_paths) + ] + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + policy.set_static_paths(0, 1, bundles) + placed, _ = policy.place_demand(fg, 0, 1, 0, 10_000.0) + assert placed == pytest.approx(float(n_paths)) # all 16 units of capacity + + +class TestMaskSemantics: + def test_down_bundle_creates_no_flow_and_cost_is_post_prune(self, algs): + """A bundle whose cheapest walk is masked reports the surviving walk's + cost; a bundle with no surviving walk is down.""" + # Two parallel two-hop walks 0->1: via node 2 (cost 1+1) and node 3 (cost 5+0). + g = _graph( + 4, + [(0, 2, 5.0, 1), (2, 1, 5.0, 1), (0, 3, 5.0, 5), (3, 1, 5.0, 0)], + ) + pg = algs.build_graph(g) + cheap = [_eid(g, 0, 2), _eid(g, 2, 1)] + dear = [_eid(g, 0, 3), _eid(g, 3, 1)] + # One multi-walk bundle containing both walks: merge the two single-path + # DAGs by using an SPF-style DAG is not possible here (costs differ), so + # build the union manually via two bundles for the down test, and use the + # dear-only bundle for the cost check after masking the cheap walk. + b_cheap = ngc.PredDAG.from_edges(g, cheap) + b_dear = ngc.PredDAG.from_edges(g, dear) + + em = np.ones(4, dtype=bool) + em[cheap[0]] = False # kill the cheap walk entirely + + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig(), edge_mask=em) + policy.set_static_paths(0, 1, [b_cheap, b_dear]) + placed, _ = policy.place_demand(fg, 0, 1, 0, 100.0) + assert policy.flow_count() == 1 # cheap bundle is DOWN + ((_, flow),) = policy.flows.items() + assert flow[2] == 5 # cost of the surviving (dear) walk + assert placed == pytest.approx(5.0) + + def test_multiwalk_bundle_survives_partial_failure_with_surviving_cost(self, algs): + """An SPF DAG used as one bundle renormalizes over surviving walks and + reports the post-prune min cost (SR-TE-style semantics).""" + # Equal-cost diamond: 0->1 via 2 or via 3, all edges cost 1 cap 5. + g = _graph(4, [(0, 2, 5.0, 1), (2, 1, 5.0, 1), (0, 3, 5.0, 1), (3, 1, 5.0, 1)]) + pg = algs.build_graph(g) + algs_local = algs + _, dag = algs_local.spf(pg, 0, None, multipath=True) + + em = np.ones(4, dtype=bool) + em[_eid(g, 0, 2)] = False # one branch of the DAG fails + + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig(), edge_mask=em) + policy.set_static_paths(0, 1, [dag]) + placed, _ = policy.place_demand(fg, 0, 1, 0, 100.0) + assert policy.flow_count() == 1 + assert placed == pytest.approx(5.0) # surviving branch only + ((_, flow),) = policy.flows.items() + assert flow[2] == 2 + + def test_all_bundles_down_places_nothing_and_is_idempotent(self, algs, square): + g, pg, short, long = square + nm = np.ones(4, dtype=bool) + nm[1] = False + nm[3] = False # both intermediate nodes down + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig(), node_mask=nm) + policy.set_static_paths(0, 2, [short, long]) + for _ in range(2): + placed, left = policy.place_demand(fg, 0, 2, 0, 3.0) + assert (placed, left) == (0.0, 3.0) + assert policy.flow_count() == 0 + + +class TestPinnedInertness: + def test_reoptimize_on_each_placement_is_inert(self, algs, square): + """With reoptimization enabled (as the TE presets set), pinned flows keep + their bundles: deliberate divergence from the original Python, where + reoptimization silently rerouted pinned flows onto SPF paths.""" + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy( + algs, pg, ngc.FlowPolicyConfig(reoptimize_flows_on_each_placement=True) + ) + policy.set_static_paths(0, 2, [short, long]) + placed, _ = policy.place_demand(fg, 0, 2, 0, 3.0) + assert placed == pytest.approx(3.0) + by_creation = sorted(policy.flows.items(), key=lambda kv: kv[0][3]) + assert [v[2] for _, v in by_creation] == [2, 4] # bindings unchanged + + def test_max_path_cost_is_inert_for_pinned_bundles(self, algs, square): + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig(max_path_cost=1)) + policy.set_static_paths(0, 2, [short, long]) # costs 2 and 4, both > 1 + placed, _ = policy.place_demand(fg, 0, 2, 0, 3.0) + assert placed == pytest.approx(3.0) # pinned bundles bypass the cost gate + + def test_lifecycle_remove_and_rebalance(self, algs, square): + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + policy.set_static_paths(0, 2, [short, long]) + policy.place_demand(fg, 0, 2, 0, 3.0) + policy.remove_demand(fg) + assert np.asarray(fg.edge_flow_view()).sum() == pytest.approx(0.0) + placed, _ = policy.place_demand(fg, 0, 2, 0, 3.0) # re-pins from bundles + assert placed == pytest.approx(3.0) + placed_r, _ = policy.rebalance_demand(fg, 0, 2, 0, 1.0) + assert placed_r == pytest.approx(2.0) # 1.0 per flow target + + +class TestValidation: + def test_repin_before_placement_replaces(self, algs, square): + g, pg, short, long = square + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + policy.set_static_paths(0, 2, [short]) + policy.set_static_paths(0, 2, [short, long]) # no spurious throw + fg = ngc.FlowGraph(g) + placed, _ = policy.place_demand(fg, 0, 2, 0, 3.0) + assert placed == pytest.approx(3.0) + assert policy.flow_count() == 2 + + def test_repin_with_masks_pruning_is_not_spurious(self, algs, square): + """The design-panel blocker: derived max_flow_count must not make + re-pinning the same bundles throw once masks prune some of them.""" + g, pg, short, long = square + em = np.ones(4, dtype=bool) + em[_eid(g, 0, 1)] = False # short bundle will be down (U=1 < N=2) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig(), edge_mask=em) + policy.set_static_paths(0, 2, [short, long]) + policy.set_static_paths(0, 2, [short, long]) # must not throw + assert policy.flow_count() == 0 # no flows until placement + + def test_rejects_bundle_from_a_different_graph(self, algs, square): + g, pg, short, _ = square + other = _graph( + 4, [(0, 3, 1.0, 7), (3, 2, 1.0, 7), (0, 1, 1.0, 9), (1, 2, 1.0, 9)] + ) + foreign = ngc.PredDAG.from_edges(other, [_eid(other, 0, 1), _eid(other, 1, 2)]) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + with pytest.raises(ValueError, match="different graph|does not connect"): + policy.set_static_paths(0, 2, [foreign]) + + def test_rejects_bundle_without_src_dst_walk(self, algs, square): + g, pg, short, _ = square + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + with pytest.raises(ValueError, match="no src->dst walk"): + policy.set_static_paths(1, 3, [short]) # short connects 0->2, not 1->3 + + def test_set_after_placement_and_wrong_demand_throw(self, algs, square): + g, pg, short, long = square + fg = ngc.FlowGraph(g) + policy = ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()) + policy.set_static_paths(0, 2, [short]) + policy.place_demand(fg, 0, 2, 0, 1.0) + with pytest.raises(ValueError, match="already holds flows"): + policy.set_static_paths(0, 2, [long]) + policy.remove_demand(fg) + policy.set_static_paths(0, 2, [long]) # allowed again after removal + with pytest.raises(ValueError, match="do not match demand"): + policy.place_demand(fg, 0, 1, 0, 1.0) + + def test_config_incompatibilities(self, algs, square): + g, pg, short, long = square + with pytest.raises(ValueError, match="max_flow_count"): + ngc.FlowPolicy( + algs, pg, ngc.FlowPolicyConfig(max_flow_count=3) + ).set_static_paths(0, 2, [short, long]) + with pytest.raises(ValueError, match="shortest_path"): + ngc.FlowPolicy( + algs, pg, ngc.FlowPolicyConfig(shortest_path=True) + ).set_static_paths(0, 2, [short]) + with pytest.raises(ValueError, match="non-empty"): + ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()).set_static_paths(0, 2, []) + with pytest.raises(ValueError, match="out of range or equal"): + ngc.FlowPolicy(algs, pg, ngc.FlowPolicyConfig()).set_static_paths( + 2, 2, [short] + ) diff --git a/tests/py/test_thread_safety.py b/tests/py/test_thread_safety.py index 059d9f7..9720542 100644 --- a/tests/py/test_thread_safety.py +++ b/tests/py/test_thread_safety.py @@ -166,7 +166,10 @@ def mutate_mask(): # Should get consistent results (either with or without node 2) num_paths = results[0] - assert num_paths >= 0 # Should not crash or return corrupted data + # The mask must be copied before the GIL is released, so the run sees one + # consistent snapshot: 2 paths (mask untouched) or 1 (node 2 blocked, leaving + # 0->1->4). Any other count means it observed a torn mask. + assert num_paths in (1, 2) class TestMaxFlowThreadSafety: @@ -277,20 +280,27 @@ def test_repeated_concurrent_mutations(self): graph = algs.build_graph(g) node_mask = np.ones(n, dtype=bool) + # pytest.fail() inside a worker thread cannot fail the test: the exception + # stays in that thread and join() does not re-raise it. Record failures and + # assert on them from the main thread instead. + errors: list[str] = [] def run_algorithms(): """Run various algorithms repeatedly.""" for _ in range(20): try: algs.spf(graph, 0, node_mask=node_mask) - except Exception as e: - pytest.fail(f"Algorithm raised exception: {e}") + except Exception as e: # noqa: BLE001 - reported below + errors.append(f"algorithm raised: {e!r}") def mutate_continuously(): """Continuously mutate the mask.""" - for _ in range(100): - node_mask[:] = np.random.rand(n) > 0.5 - time.sleep(0.001) + try: + for _ in range(100): + node_mask[:] = np.random.rand(n) > 0.5 + time.sleep(0.001) + except Exception as e: # noqa: BLE001 - reported below + errors.append(f"mutator raised: {e!r}") algo_thread = threading.Thread(target=run_algorithms) mutator_thread = threading.Thread(target=mutate_continuously) @@ -301,8 +311,7 @@ def mutate_continuously(): algo_thread.join() mutator_thread.join() - # If we got here without crashes/exceptions, thread safety is working - assert True, "Thread safety test completed without crashes" + assert not errors, "; ".join(errors) class TestMemoryOrdering: