Fix TM sizing for parallel edges, version 0.3.1

networmix · networmix · commit f7fca881e8e9 · 2025-12-07T01:19:20.000-08:00
Parallel edges (striped corridors) between metro pairs now sized
independently during TM-based capacity sizing.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,18 @@
+# Changelog
+
+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.1] - 2025-12-07
+
+### Fixed
+
+- **TM Sizing**: Parallel edges (striped corridors) between metro pairs now sized independently. Previously, only one edge per metro pair was updated during TM-based capacity sizing, leaving striped links undersized.
+
+## [0.3.0] - 2025-11-XX
+
+### Changed
+
+- Initial versioned release with TM-based capacity sizing.
diff --git a/pyproject.toml b/pyproject.toml
@@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta"
 # ---------------------------------------------------------------------
 [project]
 name = "topogen"
-version = "0.3.0"
+version = "0.3.1"
 description = "A network topology generator."
 readme = "README.md"
 authors = [{ name = "Andrey Golovanov" }]
diff --git a/tests/topogen/scenario/test_graph_pipeline_unit.py b/tests/topogen/scenario/test_graph_pipeline_unit.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from types import SimpleNamespace
+from unittest.mock import patch
 
 import networkx as nx
 
@@ -15,6 +16,27 @@ def _cfg() -> SimpleNamespace:
     )
 
 
+def _tm_sizing_cfg() -> SimpleNamespace:
+    # Config with TM sizing enabled for testing
+    return SimpleNamespace(
+        traffic=SimpleNamespace(
+            enabled=True,
+            mw_per_dc_region=10.0,
+            gbps_per_mw=100.0,
+            matrix_name="default",
+        ),
+        build=SimpleNamespace(
+            tm_sizing=SimpleNamespace(
+                enabled=True,
+                quantum_gbps=3200.0,
+                headroom=1.3,
+                respect_min_base_capacity=True,
+                flow_placement="EQUAL_BALANCED",
+            )
+        ),
+    )
+
+
 def test_metro_index_and_node_id_helpers() -> None:
     metros = [
         {"name": "A", "node_key": (0.0, 0.0)},
@@ -203,3 +225,189 @@ def test_to_network_sections_serializes_groups_and_adjacency() -> None:
         )
         for a in adjacency
     )
+
+
+def test_tm_sizing_preserves_parallel_edges() -> None:
+    """TM sizing must update all parallel corridor edges, not just one.
+
+    When multiple corridor edges exist between the same metro pair (striping),
+    each edge must get its capacity sized independently based on its share of
+    the traffic load.
+    """
+    # Build a site graph with 3 parallel corridor edges between metros A and B
+    G = nx.MultiGraph()
+    metros = [
+        {"name": "A", "x": 0.0, "y": 0.0, "radius_km": 10.0, "node_key": (0.0, 0.0)},
+        {
+            "name": "B",
+            "x": 100.0,
+            "y": 0.0,
+            "radius_km": 10.0,
+            "node_key": (100.0, 0.0),
+        },
+    ]
+    metro_settings = {
+        "A": {"pop_per_metro": 1, "dc_regions_per_metro": 1},
+        "B": {"pop_per_metro": 1, "dc_regions_per_metro": 1},
+    }
+
+    # Add nodes
+    G.add_node("metro1/pop1", site_kind="pop")
+    G.add_node("metro1/dc1", site_kind="dc")
+    G.add_node("metro2/pop1", site_kind="pop")
+    G.add_node("metro2/dc1", site_kind="dc")
+
+    # Add 3 parallel inter-metro corridor edges between the same PoPs
+    original_capacity = 1000.0
+    for i in range(3):
+        G.add_edge(
+            "metro1/pop1",
+            "metro2/pop1",
+            key=f"corridor:{i}",
+            link_type="inter_metro_corridor",
+            base_capacity=original_capacity,
+            target_capacity=original_capacity,
+            cost=500,
+            source_metro="A",
+            target_metro="B",
+        )
+
+    # Add DC-to-PoP edges (required for TM generation)
+    G.add_edge(
+        "metro1/dc1",
+        "metro1/pop1",
+        key="dc_to_pop:1",
+        link_type="dc_to_pop",
+        base_capacity=original_capacity,
+        cost=1,
+        source_metro="A",
+        target_metro="A",
+    )
+    G.add_edge(
+        "metro2/dc1",
+        "metro2/pop1",
+        key="dc_to_pop:2",
+        link_type="dc_to_pop",
+        base_capacity=original_capacity,
+        cost=1,
+        source_metro="B",
+        target_metro="B",
+    )
+
+    # Mock traffic matrix to generate demands between metros
+    mock_tm = {
+        "default": [
+            {
+                "source_path": "^metro1/dc1/.*",
+                "sink_path": "^metro2/dc1/.*",
+                "demand": 10000.0,
+            },
+            {
+                "source_path": "^metro2/dc1/.*",
+                "sink_path": "^metro1/dc1/.*",
+                "demand": 10000.0,
+            },
+        ]
+    }
+
+    cfg = _tm_sizing_cfg()
+    with patch("topogen.traffic_matrix.generate_traffic_matrix", return_value=mock_tm):
+        gp.tm_based_size_capacities(G, metros, metro_settings, cfg)
+
+    # Verify ALL 3 parallel corridor edges got updated (not just one)
+    corridor_edges = [
+        (u, v, k, d)
+        for u, v, k, d in G.edges(keys=True, data=True)
+        if d.get("link_type") == "inter_metro_corridor"
+    ]
+    assert len(corridor_edges) == 3, "Should still have 3 parallel corridor edges"
+
+    # Each edge should have been sized (base_capacity increased from original)
+    updated_count = 0
+    for _u, _v, _k, data in corridor_edges:
+        if data["base_capacity"] > original_capacity:
+            updated_count += 1
+
+    # All parallel edges should be sized independently
+    assert updated_count == 3, (
+        f"Expected all 3 parallel edges to be sized, but only {updated_count} were. "
+        "Parallel edges between the same metro pair must be tracked individually."
+    )
+
+
+def test_tm_sizing_single_edge_baseline() -> None:
+    """TM sizing works correctly with a single corridor edge (no parallelism)."""
+    G = nx.MultiGraph()
+    metros = [
+        {"name": "A", "x": 0.0, "y": 0.0, "radius_km": 10.0, "node_key": (0.0, 0.0)},
+        {
+            "name": "B",
+            "x": 100.0,
+            "y": 0.0,
+            "radius_km": 10.0,
+            "node_key": (100.0, 0.0),
+        },
+    ]
+    metro_settings = {
+        "A": {"pop_per_metro": 1, "dc_regions_per_metro": 1},
+        "B": {"pop_per_metro": 1, "dc_regions_per_metro": 1},
+    }
+
+    G.add_node("metro1/pop1", site_kind="pop")
+    G.add_node("metro1/dc1", site_kind="dc")
+    G.add_node("metro2/pop1", site_kind="pop")
+    G.add_node("metro2/dc1", site_kind="dc")
+
+    original_capacity = 1000.0
+    G.add_edge(
+        "metro1/pop1",
+        "metro2/pop1",
+        key="corridor:0",
+        link_type="inter_metro_corridor",
+        base_capacity=original_capacity,
+        target_capacity=original_capacity,
+        cost=500,
+        source_metro="A",
+        target_metro="B",
+    )
+    G.add_edge(
+        "metro1/dc1",
+        "metro1/pop1",
+        key="dc_to_pop:1",
+        link_type="dc_to_pop",
+        base_capacity=original_capacity,
+        cost=1,
+        source_metro="A",
+        target_metro="A",
+    )
+    G.add_edge(
+        "metro2/dc1",
+        "metro2/pop1",
+        key="dc_to_pop:2",
+        link_type="dc_to_pop",
+        base_capacity=original_capacity,
+        cost=1,
+        source_metro="B",
+        target_metro="B",
+    )
+
+    mock_tm = {
+        "default": [
+            {
+                "source_path": "^metro1/dc1/.*",
+                "sink_path": "^metro2/dc1/.*",
+                "demand": 5000.0,
+            },
+        ]
+    }
+
+    cfg = _tm_sizing_cfg()
+    with patch("topogen.traffic_matrix.generate_traffic_matrix", return_value=mock_tm):
+        gp.tm_based_size_capacities(G, metros, metro_settings, cfg)
+
+    # The single corridor edge should be sized
+    corridor_data = G.get_edge_data("metro1/pop1", "metro2/pop1", "corridor:0")
+    assert corridor_data is not None
+    assert corridor_data["base_capacity"] > original_capacity, (
+        "Single corridor edge should have increased capacity after TM sizing"
+    )
diff --git a/topogen/scenario/graph_pipeline.py b/topogen/scenario/graph_pipeline.py
@@ -992,11 +992,15 @@ def tm_based_size_capacities(
 
     Pipeline:
     - Generate TM using traffic_matrix.generate_traffic_matrix (in-memory).
-    - Build a metro-level NetworkX graph with inter-metro corridor edges from G.
-    - Convert to netgraph_core StrictMultiDiGraph via from_networkx() with
-      bidirectional edges for symmetric flow routing.
+    - Build a metro-level NetworkX MultiDiGraph with explicit forward and reverse
+      edges for each inter-metro corridor in G. This allows traffic to flow in
+      both directions while keeping per-direction loads separate.
+    - Convert to netgraph_core StrictMultiDiGraph via from_networkx().
     - For each directed TM demand in the matrix, compute shortest-path ECMP
-      fractions and accumulate load on inter-metro corridor edges only.
+      fractions and accumulate load on inter-metro corridor edges.
+    - Track forward and reverse flows separately using different edge refs.
+      Since G is undirected, both refs point to the same G edge, but respect_min
+      ensures final capacity = max(sized_forward, sized_reverse).
     - Quantize inter-metro base capacities with headroom.
     - Derive DC->PoP and intra-metro PoP<->PoP base capacities from metro/PoP
       egress with configurable multipliers and quantization.
@@ -1043,13 +1047,18 @@ def tm_based_size_capacities(
     for idx, _ in enumerate(metros, 1):
         metro_idx_map.setdefault(f"metro{idx}", idx)
 
-    # Build temporary NetworkX DiGraph with metro nodes and inter-metro corridors
-    H = nx.DiGraph()
+    # Build temporary NetworkX MultiDiGraph with metro nodes and inter-metro corridors.
+    # MultiDiGraph is required to preserve parallel edges between the same metro pair
+    # (e.g., striped corridors with multiple links per corridor).
+    H: nx.MultiDiGraph = nx.MultiDiGraph()
     for idx in set(metro_idx_map.values()):
         H.add_node(idx)
 
-    # Map to track correspondence between H edges and G edges
-    g_edge_refs: dict[tuple[int, int], tuple[str, str, str]] = {}
+    # Map to track correspondence between H edges and G edges.
+    # Key is (src_metro_idx, dst_metro_idx, h_edge_key) to handle parallel edges.
+    # For each undirected G edge, we create two directed H edges (forward and reverse)
+    # with DIFFERENT refs so that flows in each direction are tracked separately.
+    g_edge_refs: dict[tuple[int, int, Any], tuple[str, str, str]] = {}
 
     for u_g, v_g, k_g, data in G.edges(keys=True, data=True):
         if str(data.get("link_type")) != "inter_metro_corridor":
@@ -1069,18 +1078,25 @@ def tm_based_size_capacities(
             )
 
         cost = int(data.get("cost", 1))
-        # Add edge to H with large capacity for sizing
-        H.add_edge(s_idx, t_idx, capacity=1e15, cost=cost)
-        # Track forward direction G edge reference
-        g_edge_refs[(s_idx, t_idx)] = (str(u_g), str(v_g), str(k_g))
+        # Add forward edge: source_metro -> target_metro
+        h_key_fwd = H.add_edge(s_idx, t_idx, key=f"{k_g}:fwd", capacity=1e15, cost=cost)
+        g_edge_refs[(s_idx, t_idx, h_key_fwd)] = (str(u_g), str(v_g), str(k_g))
+
+        # Add reverse edge: target_metro -> source_metro
+        # Use DIFFERENT ref (v_g, u_g, k_g) so reverse flows are tracked separately.
+        # When applied to undirected G, both refs point to the same edge but are
+        # processed separately, resulting in max(sized_forward, sized_reverse).
+        h_key_rev = H.add_edge(t_idx, s_idx, key=f"{k_g}:rev", capacity=1e15, cost=cost)
+        g_edge_refs[(t_idx, s_idx, h_key_rev)] = (str(v_g), str(u_g), str(k_g))
 
     if H.number_of_edges() == 0:
         raise ValueError(
             "TM sizing: no inter-metro corridor edges present in site graph"
         )
 
-    # Convert NetworkX graph to netgraph_core format with bidirectional edges
-    multidigraph, node_map, edge_map = _from_networkx(H, bidirectional=True)
+    # Convert NetworkX graph to netgraph_core format.
+    # bidirectional=False because we already added explicit reverse edges above.
+    multidigraph, node_map, edge_map = _from_networkx(H, bidirectional=False)
     num_nodes = multidigraph.num_nodes()
 
     # Build Core graph handle
@@ -1114,13 +1130,16 @@ def tm_based_size_capacities(
         demand_val = float(d.get("demand", 0.0))
         if demand_val <= 0.0:
             continue
-        s_idx = _parse_tm_endpoint_to_metro_idx(src)
-        t_idx = _parse_tm_endpoint_to_metro_idx(dst)
-        if s_idx is None or t_idx is None or s_idx == t_idx:
+        s_metro = _parse_tm_endpoint_to_metro_idx(src)
+        t_metro = _parse_tm_endpoint_to_metro_idx(dst)
+        if s_metro is None or t_metro is None or s_metro == t_metro:
             continue
-        if s_idx < 0 or s_idx >= num_nodes or t_idx < 0 or t_idx >= num_nodes:
+        # Convert 1-based metro indices to 0-based netgraph node indices
+        s_idx = node_map.to_index.get(s_metro)
+        t_idx = node_map.to_index.get(t_metro)
+        if s_idx is None or t_idx is None:
             raise ValueError(
-                f"TM sizing: metro index out of range (src={s_idx}, dst={t_idx}, num_nodes={num_nodes})"
+                f"TM sizing: metro index out of range (src={s_metro}, dst={t_metro}, num_nodes={num_nodes})"
             )
 
         # Compute SPF
@@ -1134,7 +1153,7 @@ def tm_based_size_capacities(
             )
         except Exception as exc:
             raise ValueError(
-                f"TM sizing: SPF failed for metro {s_idx}->{t_idx}: {exc}"
+                f"TM sizing: SPF failed for metro {s_metro}->{t_metro}: {exc}"
             ) from exc
 
         # Place flow on DAG
@@ -1151,8 +1170,8 @@ def tm_based_size_capacities(
             logger.debug(
                 "TM sizing: placed %s Gbps from metro%d->metro%d (cost=%s)",
                 f"{placed:,.1f}",
-                s_idx,
-                t_idx,
+                s_metro,
+                t_metro,
                 f"{cost:,}",
             )
         except Exception:
@@ -1170,10 +1189,17 @@ def tm_based_size_capacities(
         # Map ext_id -> H edge reference (src_idx, dst_idx, key)
         h_edge_ref = edge_map.to_ref.get(ext_id)
         if h_edge_ref:
-            src_idx, dst_idx, _ = h_edge_ref
+            src_idx, dst_idx, h_key = h_edge_ref
             # H nodes are int (metro indices), so cast is safe
             if isinstance(src_idx, int) and isinstance(dst_idx, int):
-                g_edge_ref = g_edge_refs.get((src_idx, dst_idx))
+                # Look up G edge using full (src, dst, key) tuple.
+                # Forward and reverse H edges have different refs:
+                # - Forward: (u_g, v_g, k_g)
+                # - Reverse: (v_g, u_g, k_g)
+                # This keeps flows in each direction separate. Since G is undirected,
+                # both refs point to the same physical edge, but respect_min logic
+                # below ensures capacity = max(sized_forward, sized_reverse).
+                g_edge_ref = g_edge_refs.get((src_idx, dst_idx, h_key))
                 if g_edge_ref:
                     edge_loads[g_edge_ref] = edge_loads.get(g_edge_ref, 0.0) + flow_val
 
