Audit hardening (S1+S2): numerics, newton_core refactor, solver diagnostics, validation #39
Reference in New Issue
Block a user
No description provided.
Delete Branch "fix/b1-v3-c1-quick-wins"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Audit-driven hardening of the discrete-conformal C++ port, executed as a
multi-model session (see
doc/reviewer/finding-orchestration.md): Sonnet andHaiku for well-specified work, Opus for the numerics/architecture findings and
the review gate. 301/301 CGAL tests green, all Java golden-vector parity tests intact.
Findings resolved (Sessions S1 + S2)
Correctness / numerics (Opus)
euclidean_cot_weights(sliver robustness).HardJava(default, Java-parity) |SmoothBarrier(C¹).newton_core(gradient reuse, cached symbolic factorization, loop-invariant hoist, dead-var removal).NewtonStatusenum + correct iteration count + conditioning diagnostics (sparse_qr_fallback_used,min_ldlt_pivot), propagated into the CGAL result types.Robustness / coverage / API (Sonnet + Haiku)
runtime_errorwith path).okflag exposed, honest coverage script + CI gate (ramp-up), negative tests.constexpr. M1/M2/M4 — citation fixes. (A1–A3 now merged via #36.)All Sonnet/Haiku commits were validated in an Opus review pass.
Not in this PR (tracked in
finding-orchestration.md)original Varylab/TU-Berlin authors have been contacted by email; no public
release / relicensing / CGAL submission until a written reply exists.
Validation
cmake -S code -B build-cgal -DWITH_CGAL_TESTS=ON && ctest --test-dir build-cgal -R '^cgal\.'→ 301/301.C2: Fix coverage.sh — remove || true from lcov capture/extract steps so real lcov failures are visible; empty coverage.info now exits with code 3. C3: Add coverage gate to quality-gates CI job (SKIP_COVERAGE_GATE=1 ramp-up mode until I5 is resolved — fast suite covers ~9.6% not 80%). Thresholds: 80% line / 70% branch / 90% function (agreed 2026-05-31). V1: Wrap JSON parse + field extraction in try/catch — nlohmann parse_error and type_error now surface as std::runtime_error with the file path. V2: Wrap stoi/stod in XML Solver parser — missing/non-numeric attributes throw std::runtime_error instead of leaking std::invalid_argument. V4: Validate required JSON keys (dof_vector, solver, solver.*) before access — missing field produces a clear named-field error message. I2: 6 serialization negative tests (missing file, malformed JSON, missing dof_vector, missing solver block, missing XML file, non-numeric XML attr). I3: load_mesh throws on non-triangulated (quad) mesh — covers the is_triangle_mesh guard that was previously untested. I4: spherical_hessian throws on edge DOFs — covers the logic_error guard. 290/290 tests pass (+8 new). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>The Inversive-Distance solver built its Hessian inline by full finite differences: n perturbations × a full O(F) gradient eval = O(n·F) per Newton iteration (quadratic in mesh size), with no fast path at all (api-performance audit B1, second half). Port the per-face block-FD scheme already used by HyperIdeal (Phase 9b): the gradient decomposes by face (G_v = Θ_v − Σ_{f∋v} α_v, and each face's angles depend only on its 3 vertex DOFs), so the Hessian decomposes into per-face 3×3 blocks. Cost drops to O(F) face evaluations, a ≈ n/6 speed-up. - inversive_distance_functional.hpp: add the pure 3→3 kernel inversive_distance_face_grad_contribs (returns the per-face contribution −α to G; mirrors the gradient's face-skip on ℓ²≤0 exactly). - inversive_distance_hessian.hpp (new): full-FD baseline + block-FD + sym variants, mirroring hyper_ideal_hessian.hpp. - newton_solver.hpp: drop the inline full-FD lambda; call inversive_distance_hessian_block_fd_sym. - test: InversiveDistance_BlockFDHessianMatchesFullFD cross-validates the two Hessians entry-wise on a perturbed (off-equilibrium) config. 291/291 CGAL tests pass; all Inversive-Distance convergence tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Builds on the newton_core refactor; all three findings live in exactly that code. I1 (test-coverage): add NewtonStatus { Converged, MaxIterations, LinearSolverFailed, LineSearchStalled } and a `status` field to NewtonResult, so the three non-convergent exits that `converged == false` previously conflated are now distinguishable. `converged` is kept (== status==Converged) for back-compat; + to_string(NewtonStatus) for logs/tests. newton_core sets the status at each exit point (centralised by the H2 refactor). H1 (test-coverage): set res.iterations explicitly at the LinearSolverFailed and LineSearchStalled breaks (= completed steps), instead of relying on the last successful iteration's stale value. N7 (numerical-stability): surface linear-algebra conditioning in the result — `sparse_qr_fallback_used` (any iteration fell back to SparseQR) and `min_ldlt_pivot` (smallest |Dᵢᵢ| of the last LDLT, a cheap near-singularity proxy). This catches the silent case the audit flagged: on a gauge-singular Hessian SimplicialLDLT "succeeds" with a ~0 pivot and no fallback fires — now min_ldlt_pivot is tiny and observable. Tests (+3 synthetic newton_core status/diagnostic tests; +1 assertion on the closed-mesh-no-pin fallback test). All purely additive — no control-flow or convergence behaviour changes; 301/301 CGAL tests pass incl. Java parity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>957f506)Root cause of the test-fast failures (confirmed via a pure-main probe PR that failed identically): the fast job builds with -j$(nproc) inside an 800m-memory container, and parallel Eigen+GoogleTest compilation OOM-kills cc1plus ("Killed signal terminated program cc1plus") non-deterministically — unrelated to branch content (only constants.hpp reaches the fast target's header closure on this branch, with trivial constexpr additions). Serialise the fast build (-j1), mirroring the Pi-protection already applied to the test-cgal job. Fixes test-fast on main and on this PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>