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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
The dev branch has been removed (trunk-based workflow: push to main, use
short-lived feature branches + PRs for risky/reviewable changes). These three
workflows still listed 'dev' as a push trigger — a dead reference now that the
branch is gone. Removed so the CI config matches reality.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a comprehensive 'External audit documents' section to the reviewer index:
- groups all 12 audits (correctness, API/perf/packaging, robustness)
- surfaces G0 (porting rights) as the cross-cutting release blocker
- records the decisions taken: coverage gate 80/70/90, API naming A1-A5
(A1-A3 implemented in PR #36, A4-A5 deferred pending G0/G1)
- points a new session at the low-risk quick-win findings
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Checks code/docs against the published mathematics (vs java-port-audit which
checks code vs Java):
- M1: Kolpakov-Mednykh volume formula used in code but missing from references.md
- M2: BPS circle-packing paper cited as '2010' in code vs '2015' in references.md
- M3: post-2023/preprint citations back unimplemented phases — re-verify + clarify
scope before submission
- M4: no canonical citation-key convention
- M5: large derivations numerically validated but prose unreviewed — expert sign-off
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the dimensions the prior audits did not cover:
- numerical-stability: tolerance hierarchy (FD-step vs Newton tol), discontinuous
clamps, cancellation in cotangent/area, magic constants.
- input-validation: malformed JSON/XML deserialization, NaN/Inf vertex coords on
load, schema-drift handling.
- thread-safety: no mutable static state (good); undocumented same-mesh concurrency
contract; Eigen threading.
- dependency-license: existing THIRD-PARTY-LICENSES matrix is sound but predicated
on the MIT premise that G0 undermines; test meshes have no provenance/license.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every container now has an explicit --memory limit so the OOM killer
has clean boundaries to work with instead of letting any single job
consume all available RAM:
test-fast: 800m (build Eigen+GTest, run 26 tests)
test-cgal: 2000m (LOW_MEMORY_BUILD, was already set)
quality-gates: 600m (apt install + 4 script checks)
doc-build: 800m (Doxygen HTML generation)
markdown-links: 400m (pure Python, very lightweight)
memory-swap = 1.5× memory in each case (150% total) so the OOM
killer fires cleanly without exhausting the SD card.
⚠ This is workflow-side protection only. The runner-side fix
(capacity: 1 in act-runner config) prevents parallel containers
entirely and is the more reliable protection — see CLAUDE.md.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Pi runner (3-4 GB RAM, swap heavily loaded) OOMs when /ci-all
triggers test-cgal + quality-gates + doc-build + links simultaneously:
four Docker containers start at once after test-fast completes, each
consuming 150-400 MB, exhausting available RAM.
Fix: /ci-all removed from all three workflow files. Each keyword
now triggers exactly one job — no parallel container competition.
Safe workflow: one keyword per commit.
Typical sequence:
git commit -m 'fix: ... /test-cgal' → CGAL tests (~5 min)
git commit -m 'chore: /quality-gates' → style checks (~30 s)
CLAUDE.md: CI table updated with Pi-runner warning note.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-U6 from doc/reviewer/usability-audit-2026-05-31.md.
The Finding-D fix (external-audit-2026-05-30) added a clean one-call
gauge-vertex overload:
assign_euclidean_vertex_dof_indices(mesh, maps, gauge_vertex)
But all user-facing code still showed the old verbose manual loop:
auto vit = mesh.vertices().begin();
maps.v_idx[*vit++] = -1;
int idx = 0;
for (; vit != mesh.vertices().end(); ++vit) maps.v_idx[*vit] = idx++;
Replaced in three places:
README.md 'Minimal usage' code block
example_euclidean.cpp Step 3
example_layout.cpp pin_first() helper
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
U4 (README.md:17) — status line: v0.9.0 → v0.10.0, test count 0 → 277
U5 (Discrete_conformal_map.h:6-16) — \file Doxygen block rewritten:
was: 'provides a single function … Spherical/hyperbolic scheduled for Phase 8b.2'
now: lists all three discrete_conformal_map_* functions already present,
plus pointers to the circle-packing companion headers
U7 (getting-started.md) — new 'Mode 4 — Low-memory build' section added
after Mode 3; shows CONFORMALLAB_LOW_MEMORY_BUILD=ON with -j1 and explains
the -O0 / no PCH / batch-1 tradeoffs for Raspberry Pi / ≤ 4 GB runners
U8 (README.md compile-time modes) — LOW_MEMORY_BUILD entry added to the
compile-time workflow code block with a one-line explanation and the
mandatory -j1 note
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-U3 from doc/reviewer/usability-audit-2026-05-31.md.
Three concrete errors corrected:
1. check_gauss_bonnet() row (was: implies all map types work)
→ now: 'EuclideanMaps/SphericalMaps only' — HyperIdealMaps overload
is = delete (compile error since external-audit Finding-B fix)
2. enforce_gauss_bonnet() row (same issue)
→ same restriction note added
3. newton_hyper_ideal() preconditions (was: 'lambda0 initialised')
→ WRONG: HyperIdeal computes lengths from b_v, a_e via zeta functions
internally; lambda0 plays no role. Correct precondition:
'no lambda0 needed · no GB pre-check'
→ Added explanation: strictly convex energy (Springborn 2020 Thm 1.3)
→ Newton converges for any targets without a pre-check
Gauss-Bonnet prose section also updated:
- Examples now show EuclideanMaps/SphericalMaps explicitly
- HyperIdeal compile-error note added with the correct hyperbolic
identity Σ(2π-Θ_v) - Area = 2π·χ and why no pre-check is needed
- Open-mesh note added (GB identity does not hold for boundary meshes)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
U10: CLI has no parameter reference in README or getting-started.md.
Documents all 7 current flags with defaults in the audit file so
a future session can add the table to getting-started.md.
U11: Three useful CLI gaps documented as a roadmap item:
- --tol / --max-iter: Newton solver tuning, currently hardcoded
- -g cp_euclidean / -g inversive_distance: Phase-9a models not
reachable from CLI despite being fully implemented in the library
Each item has an effort estimate (~30 min / ~2 h) and concrete
acceptance criteria so a future session can pick it up cold.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-U1 and Finding-U2 from doc/reviewer/usability-audit-2026-05-31.md.
The existing examples (example_euclidean, example_layout, example_hyper_ideal)
all used the "natural theta" pattern which makes x*=0 trivially the
equilibrium — u_v ≈ 0 everywhere, no deformation. A new user following
these examples saw solver output but not conformal geometry.
New: example_flatten.cpp
- PRIMARY USE CASE: conformally flatten a mesh to the plane
- Sets Θ_v = 2π for all interior vertices (flat target)
- Pins boundary vertices (no Gauss-Bonnet check for open meshes)
- Demonstrates non-trivial u_v (cathead.obj: range ≈ 2.96, 5 Newton iters)
- Documents the difference from "natural theta" explicitly
New: example_cgal_api.cpp
- Demonstrates CGAL::discrete_conformal_map_euclidean (Discrete_conformal_map.h)
- First runnable CGAL public API example; contrast with internal API
- Documents the "natural theta" default behaviour and explains why u_v=0
- Explains when to use CGAL API vs internal API
Both examples registered in code/examples/CMakeLists.txt and compile
cleanly with -DWITH_CGAL=ON.
Updated:
- example_euclidean.cpp: prominent "TESTING CONVENTION" warning
- example_layout.cpp: same warning on set_natural_theta helper
- doc/getting-started.md: example_flatten is now the recommended
"start here" example; note on natural-theta behaviour added
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
External documentation and usability review of v0.10.0.
9 findings covering documentation correctness, API usability,
and new-user experience.
Critical:
U1 — All examples show trivial identity map (natural theta),
not real conformal flattening — new users see no-op output
U2 — CGAL public API has zero runnable examples
Medium:
U3 — contracts.md incorrect: check_gauss_bonnet row missing
HyperIdeal restriction (deleted overload after Finding-B)
U4 — README still shows v0.9.0 (current: v0.10.0, 277 tests)
U5 — Discrete_conformal_map.h header comment describes Phase-8a
state; all 5 entry functions already implemented
U6 — New gauge-vertex overload (Finding-D) not reflected in
README or examples — old verbose loop still shown
Minor:
U7 — CONFORMALLAB_LOW_MEMORY_BUILD absent from getting-started.md
U8 — CONFORMALLAB_LOW_MEMORY_BUILD absent from README table
U9 — Layout2D.uv index semantics undocumented (v.idx() contract)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
issue_comment triggers had two problems: Gitea did not reliably fire
them, and the refs/pull/N/head checkout was fragile. Commit-message
keywords are simpler and guaranteed to work on any push event.
Trigger keywords (add anywhere in the commit message):
/test-cgal → CGAL test suite (277 tests, LOW_MEMORY_BUILD)
/quality-gates → license/codespell/shellcheck/cgal-conventions
/docs → Doxygen build + warning summary
/links → Markdown internal link check
test-fast still runs on every push (no keyword needed).
All `issue_comment` event handlers and `refs/pull/N/head` checkouts
removed from all three workflow files. review/** added to push branch
filters so this PR branch triggers normally.
CLAUDE.md CI table updated.
/test-cgal /quality-gates
Every CI job except test-fast and mirror-to-codeberg now runs only
when explicitly requested via a PR comment, instead of on every push
or PR sync. This keeps the Pi runner idle during WIP commits and lets
the author decide when to pay each job's cost.
Trigger commands:
/test-cgal → CGAL test suite (277 tests, ~5 min build + 31 s run)
/quality-gates → license/codespell/shellcheck/cgal-conventions (~30 s)
/docs → Doxygen build + warning summary (~2 min)
/links → Markdown internal link check (~10 s)
All comment-triggered jobs check:
event is issue_comment
AND comment is on a PR (issue.pull_request != null)
AND comment body contains the trigger word
AND checkout uses refs/pull/N/head (not the default branch)
Jobs that stay automatic:
test-fast — runs on every push (26 pure-math tests, < 5 s)
mirror-to-codeberg — unchanged
Jobs that keep additional triggers:
markdown-links — weekly cron (Mon 05:00 UTC) + workflow_dispatch
doc-build — workflow_dispatch (for manual runs outside a PR)
quality-gates drops `needs: test-fast` — it now runs independently
when comment-triggered (caller decides whether test-fast passed first).
CLAUDE.md CI pipeline table updated with all five jobs and their new
trigger descriptions.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of triggering on every pull_request push, test-cgal now fires
only when a PR comment contains "/test-cgal". This prevents the Pi
runner (3-4 GB RAM, swap constantly loaded) from queuing CGAL builds
faster than it can drain them on every WIP commit.
Workflow changes:
- Add `issue_comment: types: [created]` to the top-level `on:` block
- test-cgal `if:` condition:
event is issue_comment
AND comment is on a PR (issue.pull_request != null)
AND comment body contains "/test-cgal"
- Checkout uses `refs/pull/N/head` so the PR branch is checked out
correctly (issue_comment sets GITHUB_REF to the default branch, not
the PR branch)
Usage: write "/test-cgal" as a PR comment to trigger the 277-test
CGAL suite. The build uses CONFORMALLAB_LOW_MEMORY_BUILD=ON (-O0,
no PCH, unity batch 1) and runs in a 2000 MB container.
CLAUDE.md: CI table + status paragraph updated accordingly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-I from doc/reviewer/external-audit-2026-05-30.md.
Root cause: CGAL+Eigen at -O3 drives cc1plus peak RAM to ~700 MB per
Unity compilation unit (batch size 4) on ARM64. With the 1600 MB
container limit the 2nd or 3rd TU reliably triggered OOM-kill, so
test-cgal was gated off via `if: false` since 2026-05-26.
Fix: new CMake option CONFORMALLAB_LOW_MEMORY_BUILD=ON applies four
orthogonal memory-saving measures to conformallab_cgal_tests:
1. -O0 (no debug info): optimizer passes entirely skipped → cc1plus
peak drops from ~700 MB to ~150-200 MB per TU on ARM64.
Omitting -g avoids the additional object-file / linker RAM cost.
2. CONFORMALLAB_USE_PCH=OFF: saves the one-time ~200 MB PCH
compilation cost; each TU re-parses CGAL headers (fast at -O0).
3. UNITY_BUILD_BATCH_SIZE=1: one source file per cc1plus invocation,
removing the "4-file template-explosion" per-unit multiplier.
4. -Wl,--no-keep-memory (GNU ld): linker releases symbol tables after
each input file → ~15-25 % less linker RSS.
Verified locally with cmake -DCONFORMALLAB_LOW_MEMORY_BUILD=ON:
277/277 CGAL tests pass, 31 s runtime (vs 2 s at -O3 — expected;
tests run 15× slower without optimizer but all correct).
CI workflow changes (cpp-tests.yml):
- test-cgal re-enabled: `if: github.event_name == 'pull_request'`
- Configure step adds -DCONFORMALLAB_LOW_MEMORY_BUILD=ON
- Container memory: 1600m → 2000m (--memory-swap=3000m for 1 GB swap
headroom), using ~half of the Pi's 3-4 GB while leaving OS margin.
CLAUDE.md updated: new flag added to compile-time options table; CI
status row corrected from DISABLED to active.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MINOR-1 (spherical_functional.hpp:426)
Wrong comment said "second derivative < 0 for a convex functional".
The spherical energy is *concave* (NSD Hessian); the monotone-f argument
applies to both convex and concave functionals equally. Comment rewritten
to explain the actual physics: increasing scale increases all angles and
thus reduces Σ G_v.
MINOR-2 (spherical_functional.hpp:494-495)
Forward finite difference O(ε) → central finite difference O(ε²):
old: dft = (sum_Gv(t + fd_eps) - ft) / fd_eps
new: dft = (sum_Gv(t + fd_eps) - sum_Gv(t - fd_eps)) / (2*fd_eps)
Same cost when the extra sum_Gv(t - fd_eps) replaces the cached ft.
MINOR-3 (euclidean_functional.hpp, spherical_functional.hpp,
inversive_distance_functional.hpp)
New header gauss_legendre.hpp centralises the 10-point Gauss-Legendre
nodes and weights (gl10_nodes() / gl10_weights()). The three energy
functions now use the shared accessors instead of duplicated local
static arrays.
MINOR-4 (euclidean_functional.hpp, spherical_functional.hpp,
hyper_ideal_functional.hpp, inversive_distance_functional.hpp)
halfedge_to_index() centralised in conformal_mesh.hpp. All four local
aliases (eucl_hidx, spher_hidx, hidx, id_detail::hidx) now delegate to
it as one-line wrappers; the aliases are kept for now to avoid a larger
call-site churn, clearly documented as thin wrappers.
MINOR-5 (clausen.hpp:33-38)
Added a comment above inits() explaining the intentional off-by-one
return value and how it interacts with csevl() — matching the Java
Clausen.inits() / csevl() contract.
277/277 CGAL + 26/26 pure-math tests pass, 0 failed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-E from doc/reviewer/external-audit-2026-05-30.md.
Scan also uncovered the same issue in inversive_distance_functional.hpp.
Three functions used absolute error `|analytic - fd| > tol` while the
rest of the library (euclidean_functional, spherical_functional,
euclidean_hessian, hyper_ideal_functional) all use relative error
`|analytic - fd| / max(1, |analytic|) > tol`.
Absolute error is too strict for large gradients (false failures) and
too lenient for small gradients.
Fixed:
cp_euclidean_functional.hpp gradient_check_cp_euclidean()
cp_euclidean_functional.hpp hessian_check_cp_euclidean()
inversive_distance_functional.hpp gradient_check_inversive_distance()
All three now use the relative criterion and accumulate all failures
before returning (ok=false instead of early return on first mismatch).
Default tol updated from 1e-6/1e-5 to 1e-4, matching the Java
FunctionalTest convention used by all other checks in the library.
Error message updated to print rel-err instead of raw diff.
266/266 CGAL tests pass, 0 failed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-D from doc/reviewer/external-audit-2026-05-30.md.
All three DOF-assignment functions iterated over every vertex
unconditionally, overwriting any v_idx set before the call. The
documentation said "pin before OR after" — the "before" option was
silently wrong (the pin would be overwritten). For the
inversive-distance variant the doc explicitly said the pre-call pin
would make the function "a no-op for that vertex", which was false.
Changes (three headers):
- euclidean_functional.hpp assign_euclidean_vertex_dof_indices()
- spherical_functional.hpp assign_vertex_dof_indices()
- inversive_distance_functional.hpp assign_inversive_distance_vertex_dof_indices()
For each:
1. Single-arg overload: doc corrected to "pin AFTER, not before;
pre-call pins are overwritten"
2. New two-arg overload accepting a Vertex_index gauge: pins the
requested vertex (v_idx=-1) in a single pass, preventing the
user error entirely
Three new GTests in test_euclidean_functional.cpp:
SingleArg_PinBeforeHasNoEffect — documents the old pitfall
TwoArg_GaugeIsPinnedOthersAreSequential — verifies the new overload
TwoArg_NewtonConvergesWithGaugeOverload — end-to-end correctness
266/266 CGAL tests pass, 0 failed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-C from doc/reviewer/external-audit-2026-05-30.md.
The box comment at the top and the function-level comment above
euclidean_cot_weights() both stated:
cot_k = (t_adj1·l123 − t_adj2·t_opp) / denom2 ← WRONG
The correct formula (verified numerically on a 3-4-5 right triangle,
expected cot1=4/3, cot2=3/4, cot3=0) is:
cot_k = (t_opp · l123 − t_a · t_b) / denom2
where t_opp is the t-value of the edge OPPOSITE vertex k, and t_a/t_b
are the t-values of the two edges ADJACENT to vertex k.
The implementation in euclidean_cot_weights() was already correct;
only the documentation was wrong.
Changes (documentation only, zero code changes):
- Box comment: rewritten with correct formula and explicit per-vertex
assignment (cot1: t_opp=t23, t_a=t12, t_b=t31; etc.)
- Box comment: added missing ½ factor to Hessian contribution lines
- Function-level comment: corrected to (t_opp·l123 − t_a·t_b)/(8·Area)
with a pointer to the box comment for the full assignment
- Inline return comment in euclidean_cot_weights(): now shows the
mapping (cot1: t_opp=t23, t_a=t12, t_b=t31) directly at the formula
263/263 CGAL tests pass, 0 failed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-B from doc/reviewer/external-audit-2026-05-30.md.
The Euclidean Gauss–Bonnet identity Σ(2π−Θ_v) = 2π·χ is ONLY valid for
flat/Euclidean and spherical metrics. For hyperbolic metrics (HyperIdeal)
the correct identity is Σ(2π−Θ_v) − Area(M) = 2π·χ. The previously
provided gauss_bonnet_sum(HyperIdealMaps) overload would silently pass the
wrong LHS to check_gauss_bonnet, which would always throw "deficit = ±Area"
for valid hyperbolic targets.
Fix:
- gauss_bonnet_sum(mesh, HyperIdealMaps) → = delete + explanation comment
- enforce_gauss_bonnet(mesh, HyperIdealMaps&) → = delete + explanation comment
- Header box comment rewritten with the correct hyperbolic Gauss–Bonnet
identity and a clear "HyperIdeal: NOT SUPPORTED" section
New test in test_phase6.cpp:
- HyperIdeal_EuclideanSumDiscrepancy_DocumentsWhyCheckIsDeleted
verifies numerically that the Euclidean sum = 0 but 2π·χ = 4π for a
regular tetrahedron, documenting the −4π discrepancy that motivated
the deletion
- Three compile-time static_asserts (SFINAE) confirm the overload is not
invocable with HyperIdealMaps but remains so with Euclidean/SphericalMaps
263/263 CGAL tests pass, 0 failed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Follow-up to the Finding-A fix (face_energy guard). The throw is the
correct safe behaviour for now; the mathematically complete solution
requires formulas for semi-ideal and fully-ideal tetrahedra that are
absent from the Java reference.
Adds a structured research entry to research-track.md (Phase 9b+):
- primary references: Kolpakov-Mednykh arXiv:math/0603097, Milnor 1982,
Vinberg 1985
- acceptance criteria: two new volume functions + gradient-check tests +
limiting-behaviour continuity witness
- explicit note that the throw in face_energy() must not be removed
without implementing and testing the replacement formulas
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Finding-A from doc/reviewer/external-audit-2026-05-30.md.
Root cause: both the Java reference (HyperIdealFunctional.java:222-231)
and the C++ port silently applied the one-ideal-vertex volume formula
to the first ideal vertex found in a face, ignoring any additional ideal
vertices. For two or three ideal vertices this produces a wrong energy
value with no diagnostic.
Fix: add an ideal_count guard at the top of face_energy() that throws
std::logic_error for ideal_count >= 2. The one-ideal (Kolpakov-Mednykh)
and zero-ideal (Meyerhoff/Ushijima) paths are unchanged and correct.
Three new GTests cover the three guard cases:
MultiIdealGuard_TwoIdealVertices_Throws (two ideal → throw)
MultiIdealGuard_AllThreeIdealVertices_Throws (all ideal → throw)
MultiIdealGuard_ExactlyOneIdeal_DoesNotThrow (one ideal → no throw)
262/262 CGAL tests pass, 0 failed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Full external code review of v0.10.0 against all math-critical headers
(direct source read, not automated scan). Documents 5 open bugs/API errors,
3 test gaps inherited from java-port-audit.md, 1 architectural CI risk,
and 5 minor findings. Each finding is self-contained with file+line
references, a concrete fix proposal, and acceptance criteria so a new
session can pick up any single finding without prior context.
Key findings:
- FINDING-A (critical): face_energy() wrong for mixed ideal/hyper-ideal configs
- FINDING-B (medium): Gauss–Bonnet API conceptually wrong for HyperIdeal
- FINDING-C (medium): cotangent formula in euclidean_hessian.hpp header is wrong
- FINDING-D (medium): DOF-assignment doc falsely claims "pin before" works
- FINDING-E (medium): cp_euclidean gradient_check uses absolute not relative error
- FINDING-F/G/H: three open test gaps from java-port-audit.md
- FINDING-I (arch): 246/272 CGAL tests not gated in CI
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
