test(sae): #2720 chart-gauge orbit baseline re-measurement at current main - #2770
Draft
HomunculusLabs wants to merge 2 commits into
Draft
test(sae): #2720 chart-gauge orbit baseline re-measurement at current main#2770HomunculusLabs wants to merge 2 commits into
HomunculusLabs wants to merge 2 commits into
Conversation
… current main Re-measures |gᵀvᵢ| projections of the KKT gradient onto the chart-gauge basis directions at the ard_saddle_state fixture (SauersML#2336's geometry). Pre-a386c1e8b baseline: v₀ = 8.24× tol, v₁ = 10.45× tol Post-a386c1e8b (this measurement): v₀ = 0.43× tol, v₁ = 1.89× tol The solver change materially shifted the stall state. v₀ dropped below tolerance entirely; v₁ dropped from 10.45× to 1.89× but remains above. The modelling question is still live but the witness is weaker.
…isite failures Engineer (gpt-5.6-sol) + oracle review found: - Every failure path (inner solve Err, assemble failure, empty gauge basis) silently returned Ok, making CI green with no indication nothing was measured - The test ran a 40-iteration solve on every suite invocation but provided no regression protection since it never asserted Changes: - Mark #[ignore] with run instructions: --ignored --nocapture - Replace all silent-return failure paths with .expect() / panic! - Assert grad_norm, tolerance, and per-direction projections are finite - Assert gauge_basis is non-empty (for this fixture, empty = drift) - Simplified tolerance branch (tolerance now asserted > 0, so the INFINITY fallback is unreachable and removed) Verified: compiles clean, shows as ignored in normal suite, runs and passes all assertions with --ignored --nocapture. Measurements reproduced: v0=0.43× tol, v1=1.89× tol (matching PR description).
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.
What
Re-measures the
|gᵀvᵢ|directional derivatives of the penalized objective along the chart-gauge orbit directions at theard_saddle_statefixture (#2336 geometry), which is the same fixture the original8.24×/10.45×measurement was taken on.Why
The issue body and three comment threads warned that the acceptance criteria numbers (
8.24×and10.45×over tolerance) were measured at a stall state produced by a solver that no longer exists —a386c1e8bdeletedexact_hessian_physical_complement(zero remaining references crate-wide), which made the Newton step minimum-norm on a complement holding 87–93% of the gradient. 18 of #2674's 19 fixtures flipped green on that change.This PR takes that measurement.
Measurement
a386c1e8ba386c1e8b(this PR)4.592e-3(8.24× tol)2.403e-4(0.43× tol — below!)5.823e-3(10.45× tol)1.067e-3(1.89× tol)5.575e-45.643e-4What changed
8.24×/10.45×targets is no longer meaningfulWhat is NOT invalidated
1.29e-16 … 1.11e-15over 1333 constructions) and not of the posterior. That is a statement about the objective and no solver change can touch it.dense_step_gauge_vector_from_fieldinfit_drivers.rswas not touched bya386c1e8b.tests_gauge_frame_roundtrip_2720.rsreads the same untouched file.quotient_residual_norm_sqwas not touched. The second disjunct ofquasi_laplace_kkt_stationaryremains an unguarded weakening in a direction the solver is structurally biased to satisfy.The
-4.054warning does not applyThe #2674 lane warned that anything computed against
log|A|on the old quotiented block is short by-4.054. These are|gᵀv|projections of the raw gradient onto a fixed basis, not log-determinants.exact_joint_chart_gauge_basisis unchanged and retained explicitly as diagnostic-only.Method
The test creates the
ard_saddle_statefrom #2336, runspenalized_quasi_laplace_criterion_with_cacheto reach the inner solve state, assembles the Arrow-Schur system, extracts the joint KKT gradient[g_t (per row); g_β], gets the chart-gauge basis fromjoint_chart_gauge_basis_for_arrow_layout, and projects|gᵀvᵢ|againstSAE_MANIFOLD_INNER_GRAD_REL_TOL * iterate_scale.The test is marked
#[ignore]— it is a manual diagnostic reproducer, not a regression gate. Run with:Prerequisite failures (inner solve refusal, assembly failure, empty gauge basis) panic rather than silently passing, so fixture drift is caught immediately.
Review
Reviewed through the RepoPrompt pipeline (engineer agent + oracle review):
converged_state_with_residual_a_saddle_2336in construction_tests.rs), gradient layout (matches ArrowSchurSystem ordering). Identified P2: all failure paths silently returned Ok, making the test always-green. Identified that the 40-iteration solve should not run in normal CI.#[ignore]for the manual reproducer, panicking on prerequisite failures, and asserting finiteness of gradient/tolerance/projections. Applied.Not established
gamma_fd_tiny_fixture(K=2, d=1, n=10, p=3, m=3 Periodic atoms) with ARD saddle perturbation — same as SAE dense route: terminal exact-curvature saddle-escape (B-converged modes can be A-saddles) #2336. Whether other chart kinds (Euclidean, Poincaré, Duchon) or framed atoms show different behavior is unmeasured.‖g‖ = 1.19e-3). The pre-a386c1e8bmeasurements were taken at a stall state where the solver could not reduce the orbit component; whether a fixture exists where the current solver also stalls with materially higher orbit derivatives is an open question.