Fix benchmark validity: penalty bound, nontrivial instances + raw metrics, embedder refinement - #10
Merged
Merged
Conversation
… drop flights Dropping a flight saves its full option cost (~c_max), not the cost spread, so A = 2*(1 + c_max - c_min) made infeasible ground states optimal whenever spread << cost (alpha=0: 40/40 seeds infeasible). Regression test covers the exact broken case.
…tric - problem.py: 5 blobs spanning 2-3 levels, central-band routes, wider blobs. F=4 conflict counts: median 3->6, zero-conflict seeds 3/20->0/20. - exact.py: raw_metrics (unrepaired QUBO energies — repair cannot game them), solve_greedy names the repair heuristic as an honest competitor, repair gains deterministic promote-and-retry (0/2000 failures at the new density; single pass failed on dense seeds). - run.py: --min-conflicts substitution (visible, deterministic, seed+1000), raw_best_E/raw_mean_E columns, primary raw-energy + secondary repaired-cost summary tables.
…to geometry - refine_embedding: vectorized force-directed repair (springs on missing edges, repulsion on spurious/crowded pairs), per-node displacement cap (uncapped sums diverged to inf — and NaN layouts passed check_embedding as 'valid'), antisymmetric directions for coincident nodes, seeded annealing kicks to escape tug-of-war equilibria, stagnation early-out. - embed(): greedy init + 8 seeded jittered restarts; single entry point for both the solver register builder and the study. - check_embedding: vectorized; non-finite coordinates now report invalid. - Study: degree computed on the independence graph (the graph actually embedded), logs n_nodes_deg_ge_6, n_restarts_used, refine_iters, and n_all_level_pairs — two flights conflicting at every level form a triangular prism that provably cannot satisfy the margin rules in 2D, a low-degree geometric obstruction the degree diagnostic misses. - Tests: 5-leaf star embeds, 7-leaf star still fails (geometry), embed deterministic per seed, refinement strictly reduces violations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three scientific problems found when running the minimal pipeline, fixed in dependency order (Fix 2 → Fix 1 → Fix 3). All measured numbers below are reproducible with the seeds shown.
Fix 2 — penalty bound was mathematically wrong
A = B = 2·(1 + (c_max − c_min))claimed feasible ground states, but skipping a flight saves its full option cost, not the spread. Verified:make_scenario(4, 3, seed, alpha=0.0)gave infeasible brute-force ground states in 40/40 seeds. With the corrected boundA = B = 2·(1 + c_max): 0/40. Regression testtest_ground_state_feasible_small_spreadcovers the exact broken case.Fix 1 — benchmark could not distinguish solvers
1a. Trivial instances. Generator tuned (5 blobs spanning 2–3 levels, up to 4 columns wide, routes drawn from the central row band). Conflict-edge counts at F=4, K=3, seeds 0..19:
[0,0,1,10,1,2,4,3,3,2,2,6,6,0,5,12,6,2,3,8][4,3,4,14,6,11,2,12,8,3,3,6,12,1,6,15,6,10,3,10]Target (median ≥ 4, ≤ 1/20 zeros) met.
run.py --min-conflicts N(default 1) additionally skips sub-threshold scenarios with a printed notice and deterministic seed+1000 substitution recorded in the CSV.1b. Repair masked all quality differences. Raw QUBO energy of unrepaired samples is now the primary metric (
raw_metrics, CSV columnsraw_best_E/raw_mean_E); the repair heuristic is exposed as the named solvergreedy. The denser instances also exposed that the one-pass repair could leave conflicts standing; it gained a deterministic promote-and-retry (0/2000 failures at the new density).python run.py --flights 5 --options 3 --seeds 10 --shots 1000now shows a spread:Solvers no longer all tie at 1.000 (greedy misses the optimum on 2/10 seeds), and the raw columns carry the real signal: the analog sampler's best raw shot equals E_min on every seed with a raw mean of ~392 vs random's ~2724. Reported as measured — nothing was tuned to favor the quantum solver.
Fix 3 — embedder strengthened; failures now attributed
embed()(greedy init → vectorized force-directed refinement → 8 seeded jittered restarts, deterministic per seed) replaces the one-pass placer everywhere (solver register builder + study). Refinement fixes found on the way: per-node displacement cap (uncapped spring sums between far clusters diverged to inf — and NaN layouts previously passedcheck_embeddingas "valid", which is why old large-F numbers looked plausible), antisymmetric force directions for coincident nodes, seeded annealing kicks for tug-of-war equilibria, stagnation early-out.max_degreeis now computed on the independence graph actually embedded;n_nodes_deg_ge_6,n_all_level_pairs,n_restarts_used,refine_itersadded to the CSV.Fit-fraction ladder, seeds 0..19 (old = one-pass greedy, new = embed(); both on the tuned generator):
Deviation from the acceptance criterion, reported as-is: "F=4 must embed at ~100%" is not reachable, and the reason is geometry, not the embedder. The tuned generator (1a) routinely makes two flights conflict at all K levels; their one-hot triangles plus the level-matching edges form a triangular prism, which cannot satisfy the margin rules (edges ≤ 0.95·R_b, non-edges ≥ 1.05·R_b, 5 µm floor) in 2D — provable for the aligned case (it would need a triangle side of ≥ 11.8 µm against a 7.6 µm ceiling) and confirmed by 500-restart searches (never below 6 violations). The 3 "unexplained" F=4 failures are cross-flight conflict triangles at shared levels — same obstruction family; each also resisted 200-restart deep searches. The acceptance criterion's premise ("max degree ~5 ⇒ failures are embedder bugs") is therefore false on this ensemble; per instruction 5, nothing was tuned to make the numbers look better. The study CSV now attributes every failure (deg≥6 / prism / other), which was the criterion's actual intent.
Verification
ruff check .,mypy contrail_env,pytest(29 tests) — all greenpython run.py --flights 5 --options 3 --seeds 10 --shots 1000— spread shown abovepython -m contrail_env.embedding_study --csv embedding.csv --seeds 20— ladder above, 7 min 44 s total