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>
