Placement semantics: pooled combine mode, lossy ECMP, resolution-based MSD (0.23.0) - #111
Conversation
…d MSD Demand placement fixes and the models they rest on: - Combine mode is a virtual source: with hop-by-hop presets every source that can reach a target originates an even share (fan-out DAG built with Core's reverse SPF), and under SHORTEST_PATHS_ECMP the pool is admitted as one demand at a single lossless scale. TE presets keep letting capacity decide which sources originate. - SHORTEST_PATHS_ECMP models a load-blind forwarding table (Core EQUAL_BALANCED_FIXED); new SHORTEST_PATHS_ECMP_LOSSY delivers what survives per-link drops and reports dropped_edges. - One preset_config feeds both placement engines; hop-by-hop FlowPolicies are cost-only and single-pass, matching the cached engine. - TE rerouting is bounded by the edge count instead of a literal 100. - MSD feasibility is judged at the engine's resolution (1/4096) and requires every demand to place something. - TrafficMatrixPlacement resolves parallelism "auto" to 1 unless an LSP preset is present or the interpreter is free-threaded. Documentation reviewed for plain language and current semantics; examples extended with demand placement, failure Monte Carlo, and result reading, all verified against the code. Minimum netgraph-core is 0.9.0. Version 0.23.0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 716cd4d636
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| volume: below the resolution the engine cannot evaluate it, and above | ||
| it nothing was carried. | ||
| """ | ||
| return self.max_shortfall <= FLOW_RESOLUTION and self.unserved_demands == 0 |
There was a problem hiding this comment.
Derive feasibility when summary details are omitted
When callers use the public, backward-compatible two-argument constructor, such as PlacementSummary(total_demand=10, total_placed=0), both new fields default to zero and is_feasible now returns True despite no demand being placed. Either derive the fallback state from the totals or avoid defaults that silently describe an infeasible result as feasible.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
|
|
||
| cost_dist: dict[float, float] = {} | ||
| used_edges: set[str] = set() | ||
| outcome = _CachedPlacement(placed=placed) |
There was a problem hiding this comment.
Report per-edge drops for lossy static paths
When SHORTEST_PATHS_ECMP_LOSSY is combined with static_paths and include_flow_details=True, placement takes this policy branch, which initializes dropped_edges as empty and never populates it. For example, the new 50-unit two-route test records 15 units as dropped but returns no per-link attribution, contrary to the documented promise that lossy entries expose dropped_edges; collect the policy's per-edge losses here or narrow the API contract.
AGENTS.md reference: AGENTS.md:L13-L14
Useful? React with 👍 / 👎.
Summary
Demand placement fixes from the flow placement review, with the models they rest on:
SHORTEST_PATHS_ECMPthe pool is admitted as one demand at a single lossless scale. TE presets keep letting capacity decide which sources originate.SHORTEST_PATHS_ECMPmodels a load-blind forwarding table (CoreEQUAL_BALANCED_FIXED); newSHORTEST_PATHS_ECMP_LOSSYdelivers what survives per-link drops and reportsdropped_edges.preset_configfeeds both placement engines; hop-by-hop FlowPolicies are cost-only and single-pass, matching the cached engine under contention.TrafficMatrixPlacementresolvesparallelism: autoto 1 unless an LSP preset is present or the interpreter is free-threaded.netgraph-coreraised to 0.9.0 (released). Version 0.23.0.Test plan
bash .superset/workspace.sh check(1274 tests, lint, types, schemas, API reference, docs)dev/check_core_integration.shagainst NetGraph-Core 0.9.0🤖 Generated with Claude Code