Audit hardening (S1+S2): numerics, newton_core refactor, solver diagnostics, validation #39

Merged
user2595 merged 16 commits from fix/b1-v3-c1-quick-wins into main 2026-05-31 22:16:38 +00:00

16 Commits

Author SHA1 Message Date
Tarik Moussa
83a61b0228 ci: build test-fast with -j1 to avoid cc1plus OOM in the 800m container
All checks were successful
C++ Tests / test-fast (pull_request) Successful in 2m20s
C++ Tests / quality-gates (pull_request) Has been skipped
C++ Tests / test-cgal (pull_request) Has been skipped
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>
2026-05-31 23:57:36 +02:00
Tarik Moussa
2e1541be4b ci: re-trigger test-fast (Pi OOM flake; fast build unaffected by this branch)
Some checks failed
C++ Tests / test-fast (pull_request) Failing after 1m45s
C++ Tests / quality-gates (pull_request) Has been skipped
C++ Tests / test-cgal (pull_request) Has been skipped
Only constants.hpp reaches the fast target's header closure (via
hyper_ideal_utility/geometry); all heavy headers (newton_solver, the CGAL
entry points, the FD Hessians) are NOT compiled by conformallab_tests. The
test-fast failures showed non-deterministic timing (2m21s vs 1m49s) — the
signature of an OOM kill in the 800m-limited Pi container, not a code defect.
Local: 301/301 CGAL + 26/26 fast green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 23:49:41 +02:00
Tarik Moussa
5f3c6aa0d5 docs(reviewer): add ready-to-paste session prompts (S3–S6 + review gate)
Some checks failed
C++ Tests / test-fast (pull_request) Failing after 1m49s
C++ Tests / quality-gates (pull_request) Has been skipped
C++ Tests / test-cgal (pull_request) Has been skipped
session-prompts.md: one self-contained, copy-paste prompt per pending session,
each naming the model (Sonnet/Haiku/Opus), the findings + audit doc, the
build/test commands, and the branch/push/PR + review-gate workflow.  S6 carries
the G0 precondition.  Linked from finding-orchestration.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
c4442a22c3 docs(reviewer): mark S2 CGAL-result follow-up done (957f506)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
56f13d7c4f feat(cgal): propagate Newton status + conditioning diagnostics into CGAL results
Follow-up to S2: surface the new NewtonResult diagnostics through the public
CGAL result types so callers of the high-level API see them too.

- Add `CGAL::Newton_status` (alias of conformallab::NewtonStatus) and three
  fields — `status`, `sparse_qr_fallback_used`, `min_ldlt_pivot` — to
  Conformal_map_result, Hyper_ideal_map_result and Circle_packing_result.
  (sparse_qr_fallback_used already existed on Conformal_map_result but was never
  populated; it is now wired through.)
- Map them from NewtonResult at all five entry points (euclidean / spherical /
  hyper_ideal / inversive_distance / cp_euclidean).
- Test: SingleTriangleConverges now asserts status == Converged and the
  diagnostics propagate.

Additive only — existing `converged`/`iterations`/`gradient_norm` semantics
unchanged. 301/301 CGAL tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
4f8530628b docs(reviewer): mark S2 (I1/H1/N7) done in the plan + audit banners
Update finding-orchestration.md (master table I1/H1/N7 → , S2 session marked
done with the original plan folded into a <details>), README headline cells +
next-batch pointer (now S3), and the test-coverage / numerical-stability audit
banners.  S2 was implemented directly by Opus in the warm post-refactor session
(commit 90e966a); 301/301 tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
1bf1defc70 feat(solver): S2 — NewtonResult status enum (I1) + iteration fix (H1) + conditioning diagnostics (N7)
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>
2026-05-31 19:44:50 +02:00
Tarik Moussa
74d41e99a8 docs(reviewer): update audit status + add finding-orchestration plan
Session 1 (2026-05-31) resolved 26 findings across Sonnet/Haiku/Opus; 298/298
CGAL tests green.  Bring the audit docs in line with reality:

- README.md: G0 banner now records that the original authors were contacted by
  email about porting/relicensing rights (awaiting reply); headline-status cells
  updated per finding; new "Session 1 — implemented" record; link to the plan.
- Per-audit resolution banners (api-performance, numerical-stability,
  input-validation, test-coverage, math-citation, thread-safety) + G0-blocked
  banners (cgal-submission, dependency-license).
- NEW finding-orchestration.md: the meta-plan mapping every finding to a session,
  an implementing model (Haiku/Sonnet/Opus by the "how much must be understood"
  rule), and an Opus review gate after each implementation session.  Records S1
  (done) and lays out S2–S6 so the next session can be picked up cold.

Docs only — no code change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
2fc465f5cf refactor(solver): H2 unify 5 Newton loops into newton_core (+B2/B3/B4/B5)
The five DCE solvers (Euclidean, Spherical, HyperIdeal, CP-Euclidean,
Inversive-Distance) carried near-identical Newton loops differing only in the
gradient function, the Hessian function, and the spherical concave sign
(test-coverage audit H2 — "5 near-identical Newton loops").  Extract one
detail::newton_core<GradFn, HessFn>(x, grad, hess, concave, tol, max_iter);
each public solver is now a thin wrapper passing two lambdas.  This lets the
performance fixes land once instead of five times:

- B2 (api-perf): line_search now optionally returns the gradient at the
  accepted point; newton_core reuses it as the next iteration's convergence
  gradient, removing ~1 redundant full-mesh gradient evaluation per iteration.
- B4 (api-perf): NewtonLinearSolver keeps one persistent SimplicialLDLT and
  runs analyzePattern once (symbolic factorization cached across iterations),
  re-analyzing only when nnz changes (self-healing for the FD Hessians that
  prune near-zero triplets).  SparseQR fallback preserved verbatim.
- B3 (api-perf): the Euclidean has_edge_dof scan is hoisted out of the loop
  (it is layout-invariant) — now done once before newton_core.
- B5 (api-perf): the dead SimplicialLDLT variable in newton_euclidean is gone.

Behaviour is unchanged: concave spherical still factors −H and solves
(−H)·Δx = G; the merit steepest-descent still uses the true H; HardJava clamp
default preserved.  Tests (+2): NewtonCore.ReusesLineSearchGradient_B2_Convex
asserts exactly 2 gradient evals on a unit-Hessian quadratic (vs 3 pre-B2), and
ConcavePathConverges covers the −H branch directly.  298/298 CGAL tests pass,
including all Java golden-vector parity tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
202b9a108d feat(num): N3 selectable scale-floor clamp mode (HardJava | SmoothBarrier)
The hyper-ideal vertex scale b is floored to keep the geometry valid.  The
original clamp `b<0 → 0.01` mirrors the Java oracle but is only C⁰ (in fact
value-discontinuous at b=0): a Newton step crossing the feasibility boundary
hits a kink that can stall convergence (numerical-stability audit N3).

Rather than replace the Java-faithful behaviour (which would break the golden
parity tests), make the floor a selectable mode so BOTH the Java standpoint
and the clean mathematics are available:

- HyperIdealScaleClamp::HardJava (DEFAULT) — the original snap, bit-for-bit
  faithful to HyperIdealFunctional.java → all parity tests unchanged.
- HyperIdealScaleClamp::SmoothBarrier — C¹ softplus floor
  b ↦ floor + softplus_β(b−floor), β = HYPER_IDEAL_SCALE_SHARPNESS (=100);
  ≈ identity away from the floor, smooth across b=0.  Opt-in.

clamp_hyper_ideal_scale centralises the logic (also folds in the N4 nachzügler:
compute_face_angles used a bare 0.01).  The mode threads with a defaulted
trailing parameter through compute_face_angles, face_angles_from_local_dofs,
evaluate_hyper_ideal, the four hyper_ideal_hessian* variants and
newton_hyper_ideal — so every existing call site keeps HardJava behaviour.

Tests (+4): clamp-function C¹/floor/identity contract, mode-equivalence away
from the boundary, and end-to-end SmoothBarrier convergence to the same Java
golden vector (LawsonHyperIdeal).  296/296 CGAL tests pass.
Documented in doc/math/geometry-modes.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
a1a7f216e0 fix(num): N5 stable triangle area (Kahan) in euclidean_cot_weights
The cotangent weights divided by 2·√(t12·t23·t31·l123).  For a needle/cap
(sliver) triangle one of the t-values is the difference of near-equal edge
lengths → catastrophic cancellation, and the area under the sqrt loses
precision, feeding large relative error into every cotangent weight, the
Hessian, and the linear solve (numerical-stability audit N5).

Replace the area computation with Kahan's stable side-length formula
(sort a≥b≥c, evaluate ¼·√[(a+(b+c))(c−(a−b))(c+(a−b))(a+(b−c))]).  The
denominator is still exactly 8·Area for well-shaped triangles but accurate
for slivers.  The triangle-inequality guard and the cotangent numerators are
unchanged.

Test: CotWeights_SliverMatchesHighPrecisionReference cross-checks a thin
triangle (apex ≈ 0.01 rad) against a long-double law-of-cosines reference.
292/292 CGAL tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 19:44:50 +02:00
Tarik Moussa
2dc4ddcc32 perf(inv-dist): B1 port — block-FD Hessian for newton_inversive_distance
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>
2026-05-31 19:44:50 +02:00
Tarik Moussa
a5718c0326 fix(coverage+validation): C2/C3 script gate, V1/V2/V4 JSON/XML errors, I2/I3/I4 tests
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>
2026-05-31 19:44:39 +02:00
Tarik Moussa
51d9844f7a docs(references): M1/M2/M4 citation fixes (audit quick-wins)
M1: Add two missing references used in hyper_ideal_utility:
  - Kolpakov, Mednykh (2006, arXiv math/0603097) — tetrahedron volume w/ one ideal vertex
  - Meyerhoff, Ushijima (2006) — tetrahedron volume w/ three ideal vertices

M2: Clarify BPS publication year: Geometry & Topology 2015 (arXiv 2010)
  - Update references.md to note "first posted 2010"
  - Normalize all code comments from "BPS-2010" → "BPS-2015" (published version)

M4: Standardize citation format in code comments
  - Normalize all "Luo (2004)" / "Luo-2004" / "Luo's 2004" → "Luo 2004"
  - Matches references.md convention: Author Year (no parens/dashes)

282/282 tests pass. Addresses M1, M2, M4 from math-derivation-citation audit.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
2026-05-31 19:44:39 +02:00
Tarik Moussa
b0946c8704 refactor(constants): centralize magic constants from functionals (N4/N6 audit)
- Add LOG_EDGE_LENGTH_FLOOR (-30.0) for degenerate edge handling
- Add HYPER_IDEAL_SCALE_FLOOR (0.01) for negative-scale clamping
- Add ASIN_DOMAIN_GUARD (1.0 - 1e-15) for asin argument bounding
- Each constant is documented with rationale and units
- Update 4 usage sites: euclidean_functional, hyper_ideal_functional,
  spherical_functional, spherical_geometry
- Add constants.hpp include to hyper_ideal_functional and spherical_functional

282/282 tests pass. Addresses N4 (unnamed magic constants) and N6 (centralize
tolerances) from numerical-stability audit.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
2026-05-31 19:44:39 +02:00
Tarik Moussa
101f3138ce fix(solver+io): B1 block-FD Hessian, V3 NaN/Inf guard, C1 ok-flag (audit quick-wins)
B1 (api-performance): wire hyper_ideal_hessian_block_fd_sym into newton_hyper_ideal
instead of the full-FD path — 33×/1166× faster on cathead/brezel, also fixes the
FD-step vs Newton-tol accuracy floor (N1 cross-fix).

V3 (input-validation): add isfinite check on all vertex coordinates in load_mesh;
NaN/Inf now throws runtime_error at the I/O boundary before poisoning the solver.

C1 (test-coverage): expose the internal ok flag via an optional bool* parameter
on solve_linear_system so double-solver failure is no longer invisible to callers.

+5 new tests (LoadMeshThrowsOnNaN/Inf, OkFlag_True*, OkFlag_NullPointerIsSafe).
282/282 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-31 19:44:39 +02:00