Release v0.3.5: bug fix (see Changelog)

Author: networmix

CHANGELOG.md

@@ -5,6 +5,12 @@ 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.3.5] - 2026-02-08
+
+### Fixed
+
+- **K-Shortest Paths**: Fix off-by-one in Yen's prefix comparison and PredDAG CSR fill order. Paths visiting nodes out of numerical order (e.g., `[0,3,2]`) previously produced malformed predecessor DAGs.
+
 ## [0.3.4] - 2026-02-02
 
 ### Changed

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build"
 
 [project]
 name = "netgraph-core"
-version = "0.3.4"
+version = "0.3.5"
 description = "C++ implementation of graph algorithms for network flow analysis and traffic engineering with Python bindings"
 readme = "README.md"
 requires-python = ">=3.11"

tests/cpp/k_shortest_paths_tests.cpp

@@ -1,5 +1,8 @@
 #include <gtest/gtest.h>
+#include <limits>
+#include <set>
 #include "netgraph/core/k_shortest_paths.hpp"
+#include "netgraph/core/shortest_paths.hpp"
 #include "netgraph/core/backend.hpp"
 #include "netgraph/core/algorithms.hpp"
 #include "netgraph/core/strict_multidigraph.hpp"
@@ -197,3 +200,108 @@ TEST(KShortestPaths, LooplessPaths) {
     EXPECT_DOUBLE_EQ(paths[0].first[3], 3.0) << "Shortest path should have cost 3";
   }
 }
+
+TEST(KShortestPaths, OutOfOrderNodePredDAGSemantics) {
+  // Graph topology that forces paths visiting nodes out of numerical order.
+  // Path 1 (shortest): 0 -> 1 -> 2 (cost 2, in-order)
+  // Path 2 (longer):   0 -> 3 -> 2 (cost 4, visits node 3 before node 2)
+  // This was the exact bug scenario: PredDAG fill for path [0,3,2] swapped
+  // parent entries between nodes 2 and 3 when using a linear index.
+  auto g = make_square_graph(1);  // 0->1->2 (cost 2) vs 0->3->2 (cost 4)
+
+  auto items = k_shortest_paths(g, 0, 2, 5, std::nullopt, true);
+
+  ASSERT_GE(items.size(), 2) << "Should find at least 2 paths";
+
+  // Verify all PredDAGs have correct semantic structure
+  for (std::size_t p = 0; p < items.size(); ++p) {
+    const auto& [dist, dag] = items[p];
+    SCOPED_TRACE("Path " + std::to_string(p));
+
+    expect_pred_dag_valid(dag, g.num_nodes());
+    expect_pred_dag_semantically_valid(g, dag, dist);
+
+    // Source must have distance 0 and destination must be reachable
+    EXPECT_EQ(dist[0], 0);
+    EXPECT_LT(dist[2], std::numeric_limits<Cost>::max());
+
+    // Verify path can be reconstructed via resolve_to_paths
+    auto concrete = resolve_to_paths(dag, 0, 2, /*split_parallel_edges=*/true);
+    EXPECT_GE(concrete.size(), 1) << "resolve_to_paths should reconstruct at least one path";
+
+    // Verify reconstructed path is valid: consecutive nodes connected by claimed edges
+    for (const auto& path : concrete) {
+      ASSERT_GE(path.size(), 2);  // at least src and dst
+      EXPECT_EQ(path.front().first, 0) << "Path should start at source";
+      EXPECT_EQ(path.back().first, 2) << "Path should end at destination";
+      // Check edge connectivity
+      for (std::size_t i = 0; i + 1 < path.size(); ++i) {
+        auto from_node = path[i].first;
+        auto to_node = path[i + 1].first;
+        const auto& edges = path[i].second;
+        for (auto eid : edges) {
+          EXPECT_EQ(g.edge_src_view()[static_cast<std::size_t>(eid)], from_node);
+          EXPECT_EQ(g.edge_dst_view()[static_cast<std::size_t>(eid)], to_node);
+        }
+      }
+    }
+  }
+}
+
+TEST(KShortestPaths, LargerOutOfOrderTopology) {
+  // Larger graph where multiple k-shortest paths visit nodes out of numerical order.
+  // Topology (6 nodes):
+  //   0 -> 5 -> 3 -> 1 (cost 3, severely out-of-order: 0,5,3,1)
+  //   0 -> 2 -> 4 -> 1 (cost 6, also out-of-order: 0,2,4,1)
+  //   0 -> 1           (cost 10, direct)
+  std::int32_t src_arr[7] = {0, 5, 3, 0, 2, 4, 0};
+  std::int32_t dst_arr[7] = {5, 3, 1, 2, 4, 1, 1};
+  double cap_arr[7] = {1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0};
+  std::int64_t cost_arr[7] = {1, 1, 1, 2, 2, 2, 10};
+
+  auto g = StrictMultiDiGraph::from_arrays(6,
+    std::span(src_arr, 7), std::span(dst_arr, 7),
+    std::span(cap_arr, 7), std::span(cost_arr, 7));
+
+  auto items = k_shortest_paths(g, 0, 1, 5, std::nullopt, true);
+
+  ASSERT_GE(items.size(), 2) << "Should find at least 2 paths";
+
+  // Verify costs are non-decreasing
+  for (std::size_t i = 0; i + 1 < items.size(); ++i) {
+    EXPECT_LE(items[i].first[1], items[i + 1].first[1])
+        << "Paths should be sorted by cost to destination";
+  }
+
+  // The first path should be 0->5->3->1 (cost 3)
+  EXPECT_EQ(items[0].first[1], 3) << "Shortest path cost should be 3";
+
+  // Verify all PredDAGs are semantically correct and can be resolved
+  for (std::size_t p = 0; p < items.size(); ++p) {
+    const auto& [dist, dag] = items[p];
+    SCOPED_TRACE("Path " + std::to_string(p));
+
+    expect_pred_dag_valid(dag, g.num_nodes());
+    expect_pred_dag_semantically_valid(g, dag, dist);
+
+    // Source must have distance 0 and destination must be reachable
+    EXPECT_EQ(dist[0], 0);
+    EXPECT_LT(dist[1], std::numeric_limits<Cost>::max());
+
+    auto concrete = resolve_to_paths(dag, 0, 1, true);
+    EXPECT_GE(concrete.size(), 1) << "Should reconstruct at least one concrete path";
+
+    // Verify node connectivity in reconstructed paths
+    for (const auto& path : concrete) {
+      ASSERT_GE(path.size(), 2);
+      EXPECT_EQ(path.front().first, 0);
+      EXPECT_EQ(path.back().first, 1);
+      // Verify no repeated nodes (simple path)
+      std::set<NodeId> visited;
+      for (const auto& [node, edges] : path) {
+        EXPECT_TRUE(visited.insert(node).second)
+            << "Node " << node << " appears twice in path (cycle)";
+      }
+    }
+  }
+}

tests/cpp/test_utils.hpp

@@ -285,17 +285,23 @@ inline void expect_csr_valid(const StrictMultiDiGraph& g) {
   auto row = g.row_offsets_view();
   auto col = g.col_indices_view();
   auto aei = g.adj_edge_index_view();
+  auto edge_src = g.edge_src_view();
+  auto edge_dst = g.edge_dst_view();
+  const auto M = static_cast<std::size_t>(g.num_edges());
+  const auto N = static_cast<std::size_t>(g.num_nodes());
+
+  // --- Forward CSR structural checks ---
 
   // Offsets should be monotonic
-  EXPECT_EQ(row.size(), static_cast<std::size_t>(g.num_nodes() + 1));
+  EXPECT_EQ(row.size(), N + 1);
   for (std::size_t i = 0; i < row.size() - 1; ++i) {
     EXPECT_LE(row[i], row[i + 1]) << "Row offsets not monotonic at " << i;
   }
 
   // Total edges match
-  EXPECT_EQ(row[row.size() - 1], g.num_edges());
-  EXPECT_EQ(col.size(), static_cast<std::size_t>(g.num_edges()));
-  EXPECT_EQ(aei.size(), static_cast<std::size_t>(g.num_edges()));
+  EXPECT_EQ(static_cast<std::size_t>(row[row.size() - 1]), M);
+  EXPECT_EQ(col.size(), M);
+  EXPECT_EQ(aei.size(), M);
 
   // All column indices and edge indices in range
   for (std::size_t i = 0; i < col.size(); ++i) {
@@ -304,6 +310,75 @@ inline void expect_csr_valid(const StrictMultiDiGraph& g) {
     EXPECT_GE(aei[i], 0) << "Invalid edge index at " << i;
     EXPECT_LT(aei[i], g.num_edges()) << "Edge index out of range at " << i;
   }
+
+  // --- Forward CSR semantic checks ---
+
+  // Bijection: every edge appears exactly once in the CSR
+  std::vector<int> edge_count(M, 0);
+  for (std::size_t u = 0; u < N; ++u) {
+    auto s = static_cast<std::size_t>(row[u]);
+    auto e = static_cast<std::size_t>(row[u + 1]);
+    for (std::size_t j = s; j < e; ++j) {
+      auto eid = static_cast<std::size_t>(aei[j]);
+      ASSERT_LT(eid, M) << "Edge index out of range at CSR pos " << j;
+      edge_count[eid]++;
+
+      // Source consistency: edge must originate from node u
+      EXPECT_EQ(static_cast<std::size_t>(edge_src[eid]), u)
+          << "Forward CSR: edge " << eid << " in node " << u
+          << "'s adjacency has src=" << edge_src[eid];
+
+      // Destination consistency: col_indices must match edge destination
+      EXPECT_EQ(col[j], edge_dst[eid])
+          << "Forward CSR: col_indices[" << j << "]=" << col[j]
+          << " doesn't match edge_dst[" << eid << "]=" << edge_dst[eid];
+    }
+  }
+  for (std::size_t eid = 0; eid < M; ++eid) {
+    EXPECT_EQ(edge_count[eid], 1)
+        << "Edge " << eid << " appears " << edge_count[eid]
+        << " times in forward CSR (expected exactly 1)";
+  }
+
+  // --- Reverse CSR checks ---
+  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();
+
+  EXPECT_EQ(in_row.size(), N + 1);
+  for (std::size_t i = 0; i < in_row.size() - 1; ++i) {
+    EXPECT_LE(in_row[i], in_row[i + 1]) << "Reverse row offsets not monotonic at " << i;
+  }
+  EXPECT_EQ(static_cast<std::size_t>(in_row[in_row.size() - 1]), M);
+  EXPECT_EQ(in_col.size(), M);
+  EXPECT_EQ(in_aei.size(), M);
+
+  // Bijection and consistency for reverse CSR
+  std::vector<int> rev_edge_count(M, 0);
+  for (std::size_t v = 0; v < N; ++v) {
+    auto s = static_cast<std::size_t>(in_row[v]);
+    auto e = static_cast<std::size_t>(in_row[v + 1]);
+    for (std::size_t j = s; j < e; ++j) {
+      auto eid = static_cast<std::size_t>(in_aei[j]);
+      ASSERT_LT(eid, M) << "Reverse edge index out of range at CSR pos " << j;
+      rev_edge_count[eid]++;
+
+      // Destination consistency: edge must point to node v
+      EXPECT_EQ(static_cast<std::size_t>(edge_dst[eid]), v)
+          << "Reverse CSR: edge " << eid << " in node " << v
+          << "'s incoming list has dst=" << edge_dst[eid];
+
+      // Source consistency: in_col_indices must match edge source
+      EXPECT_EQ(in_col[j], edge_src[eid])
+          << "Reverse CSR: in_col_indices[" << j << "]=" << in_col[j]
+          << " doesn't match edge_src[" << eid << "]=" << edge_src[eid];
+    }
+  }
+  for (std::size_t eid = 0; eid < M; ++eid) {
+    EXPECT_EQ(rev_edge_count[eid], 1)
+        << "Edge " << eid << " appears " << rev_edge_count[eid]
+        << " times in reverse CSR (expected exactly 1)";
+  }
 }
 
 inline void expect_pred_dag_valid(const PredDAG& dag, int num_nodes) {
@@ -325,6 +400,42 @@ inline void expect_pred_dag_valid(const PredDAG& dag, int num_nodes) {
   }
 }
 
+// Extended PredDAG validation that also checks semantic correctness against a graph.
+// Verifies that each entry refers to an actual edge from parent to node, and that
+// distances are consistent (dist[v] == dist[parent] + edge_cost).
+inline void expect_pred_dag_semantically_valid(const StrictMultiDiGraph& g,
+                                               const PredDAG& dag,
+                                               const std::vector<Cost>& dist) {
+  const auto N = g.num_nodes();
+  const auto edge_src = g.edge_src_view();
+  const auto edge_dst = g.edge_dst_view();
+  const auto edge_cost = g.cost_view();
+
+  expect_pred_dag_valid(dag, N);
+
+  for (std::int32_t v = 0; v < N; ++v) {
+    auto s = static_cast<std::size_t>(dag.parent_offsets[static_cast<std::size_t>(v)]);
+    auto e = static_cast<std::size_t>(dag.parent_offsets[static_cast<std::size_t>(v) + 1]);
+    for (std::size_t i = s; i < e; ++i) {
+      auto parent = dag.parents[i];
+      auto eid = static_cast<std::size_t>(dag.via_edges[i]);
+      ASSERT_LT(eid, static_cast<std::size_t>(g.num_edges()));
+      // Edge must go from parent -> v
+      EXPECT_EQ(edge_src[eid], parent)
+          << "Node " << v << ": via_edge " << eid << " src mismatch";
+      EXPECT_EQ(edge_dst[eid], v)
+          << "Node " << v << ": via_edge " << eid << " dst mismatch";
+      // Distance consistency
+      auto d_v = dist[static_cast<std::size_t>(v)];
+      auto d_p = dist[static_cast<std::size_t>(parent)];
+      if (d_v < std::numeric_limits<Cost>::max() && d_p < std::numeric_limits<Cost>::max()) {
+        EXPECT_EQ(d_v, d_p + edge_cost[eid])
+            << "Distance inconsistency at node " << v;
+      }
+    }
+  }
+}
+
 inline void expect_flow_conservation(const FlowGraph& fg, NodeId src, NodeId dst) {
   const auto& g = fg.graph();
   auto row = g.row_offsets_view();

tests/py/conftest.py

@@ -77,9 +77,19 @@ def _to_dict(summary) -> dict[float, float]:
 
 @pytest.fixture
 def assert_pred_dag_integrity():
-    """Return an assertion helper to validate PredDAG shapes and id ranges."""
+    """Return an assertion helper to validate PredDAG shapes, id ranges, and semantics.
 
-    def _assert(g: ngc.StrictMultiDiGraph, dag: ngc.PredDAG) -> None:
+    Checks structural validity (sizes, ranges, monotonicity) AND semantic
+    correctness: each parent entry must correspond to an actual edge in the
+    graph from the claimed parent to the node, and if distances are provided,
+    the distance must be consistent (dist[v] == dist[parent] + edge_cost).
+    """
+
+    def _assert(
+        g: ngc.StrictMultiDiGraph,
+        dag: ngc.PredDAG,
+        dist: np.ndarray | None = None,
+    ) -> None:
         off = np.asarray(dag.parent_offsets)
         par = np.asarray(dag.parents)
         via = np.asarray(dag.via_edges)
@@ -95,6 +105,36 @@ def _assert(g: ngc.StrictMultiDiGraph, dag: ngc.PredDAG) -> None:
             assert int(par.min()) >= 0 and int(par.max()) < g.num_nodes()
             assert int(via.min()) >= 0 and int(via.max()) < g.num_edges()
 
+        # Semantic validation: each entry must reference an actual graph edge
+        edge_src = np.asarray(g.edge_src_view())
+        edge_dst = np.asarray(g.edge_dst_view())
+        edge_cost = np.asarray(g.cost_view())
+        for v in range(g.num_nodes()):
+            s, e = int(off[v]), int(off[v + 1])
+            for i in range(s, e):
+                parent = int(par[i])
+                eid = int(via[i])
+                # The via_edge must go from parent -> v
+                assert int(edge_src[eid]) == parent, (
+                    f"PredDAG entry for node {v}: via_edge {eid} has src="
+                    f"{int(edge_src[eid])}, expected parent={parent}"
+                )
+                assert int(edge_dst[eid]) == v, (
+                    f"PredDAG entry for node {v}: via_edge {eid} has dst="
+                    f"{int(edge_dst[eid])}, expected node={v}"
+                )
+                # Distance consistency (if distances provided)
+                if dist is not None:
+                    d_v = float(dist[v])
+                    d_p = float(dist[parent])
+                    ec = float(edge_cost[eid])
+                    if d_v < 1e18 and d_p < 1e18:  # skip unreachable
+                        assert abs(d_v - (d_p + ec)) < 1e-9, (
+                            f"PredDAG distance inconsistency at node {v}: "
+                            f"dist[{v}]={d_v} != dist[{parent}]={d_p} + "
+                            f"cost({eid})={ec} = {d_p + ec}"
+                        )
+
     return _assert