Bundles the 2026-05-29 Java↔C++ math-correctness audit (doc/reviewer/ java-port-audit.md, 11 findings) with two follow-up fixes. Audit code changes: - Finding 3 (spherical_functional): edge-DOF replacement parameterization via spher_eff_lambda; edge gradient α_opp⁺+α_opp⁻−θ_e (drops additive −(S⁺+S⁻)/2) - Finding 4 (spherical_hessian): always-compiled edge-DOF throw guard - Finding 6 (period_matrix): faithful normalizeModulus (0≤Re≤½, Im≥0, |τ|≥1) - Finding 9 (inversive_distance): degenerate-face limiting angles, no skip - Findings 1/2 (euclidean): degenerate gradient limiting angles + Hessian guard Euclidean holonomy/τ fix: develop the cut surface across the dual spanning tree only (cut_graph now exposes is_dual_tree), so genus-1 cut edges yield non-degenerate lattice generators. Previously τ came out 0 / NaN / 1e13 on the bundled tori; now matches the analytic revolution modulus i·√(R²−r²)/r. Re-enabled τ reporting in the Euclidean CLI; rewrote validation.md §3/§4 accordingly. Tests (240 CGAL, 0 skipped): - HolonomyEndToEnd ×3 — tori of revolution (4×4, hex 6×6, 8×8) vs analytic modulus - SphericalFunctional.EdgeGradient_RegularTetClosedForm — independent closed-form π/3 oracle locking the Finding-3 edge formula (the path-integral FD check cannot detect a wrong-but-conservative gradient) Also documents the latent spherical/hyperbolic holonomy-extraction bug (same single-development pattern, dead code today) in research-track.md (Phase 9c/10), and adds favour/normalisations to the codespell ignore list. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
15 KiB
Java → C++ Port Audit — Anomalies & Missing Test Cases
Systematic comparison of the C++ port (code/include/*.hpp) against the original
Java reference (~/Desktop/conformallab,
de.varylab.discreteconformal.functional.*). Focus: math correctness, logic, and
faithfulness to the reference.
Status legend: ✅ fixed · ⚠️ open / needs decision · ℹ️ note (no action)
Files compared (math-critical)
| Java | C++ | Verdict |
|---|---|---|
Clausen.java |
clausen.hpp |
✅ faithful (incl. inits return, clausen2, Л, ImLi2) |
EuclideanCyclicFunctional.java |
euclidean_functional.hpp, euclidean_geometry.hpp |
✅ angles + gradient assembly match; degenerate handling fixed |
— Hessian (triangleHessian/conformalHessian) |
euclidean_hessian.hpp |
cotangent + vertex block match; edge block not implemented (now guarded) |
SphericalFunctional.java |
spherical_functional.hpp, spherical_geometry.hpp |
✅ vertex-mode + edge-DOF replacement parameterization now match (Finding 3) |
HyperIdealFunctional.java |
hyper_ideal_functional.hpp, hyper_ideal_geometry.hpp |
✅ faithful (incl. degenerate (π,0,0) angles) |
HyperIdealUtility.java |
hyper_ideal_utility.hpp |
✅ ζ, ζ₁₃, ζ₁₄, ζ₁₅, both tetrahedron-volume formulas match |
CPEuclideanFunctional.java |
cp_euclidean_functional.hpp |
✅ p(), gradient, energy match |
DiscreteEllipticUtility.java |
discrete_elliptic_utility.hpp, period_matrix.hpp |
✅ normalizeModulus faithful and now wired into compute_period_matrix (Finding 6) |
HomologyUtility.java, CanonicalBasisUtility.java, SpanningTreeUtility.java |
cut_graph.hpp |
tree-cotree only; no canonical/symplectic basis ℹ️ (Finding 7) |
EuclideanLayout.java (holonomy) |
layout.hpp (detail::euclidean_holonomy) |
translation via midpoint displacement; ignores residual rotation ℹ️ (Finding 8) |
Findings
1. ✅ FIXED — Degenerate triangle handling (Euclidean + Spherical gradient)
The Java reference assigns the limiting corner angles to a degenerate (triangle-inequality-violating) face — the corner opposite the over-long edge is π, the other two 0. This is the convex C¹ extension that keeps the BPS energy well-defined on the infeasible region, so Newton can pass through a flip without stalling.
The C++ port returned {0,0,0} and skipped the whole face in the gradient.
Tell-tale: the hyper-ideal port already mirrors Java's (π,0,0) fallback —
euclidean/spherical were inconsistent oversights.
Fix: euclidean_geometry.hpp / spherical_geometry.hpp degenerate branches
now return the limiting angles (still valid=false, so the cotangent Hessian
keeps skipping — matching Java, whose triangleHessian zeroes cotangents on
degenerate faces). euclidean_functional.hpp / spherical_functional.hpp
gradients no longer skip. All 237 tests pass; solution unchanged (only affects
evaluations that previously contributed zero).
2. ✅ FIXED — euclidean_hessian missing edge-DOF guard
Doc comment claimed "the function asserts that no edge DOF is variable" — but
no such guard existed. With edge DOFs assigned
(assign_euclidean_all_dof_indices), the Java conformalHessian's edge-edge and
vertex-edge blocks are absent, so the C++ would silently build a Hessian with
all-zero edge rows/cols (singular → LDLT fails → QR least-squares garbage).
Fix: added an always-compiled throw std::logic_error guard.
3. ✅ FIXED — Spherical edge-DOF parameterization now matches Java
Java's spherical functional drops the u_i + u_j vertex terms when an edge is
a variable (replacement parameterization, SphericalFunctional.java:398–400),
whereas the C++ previously used additive Λ = λ⁰ + u_i + u_j + λ_e always. That
made the C++ spherical edge gradient carry an extra −(S_f⁺+S_f⁻)/2 term absent
from Java's G.add(i, αk + αl − π) (SphericalFunctional.java:283–292).
Fix: added helper spher_eff_lambda implementing the replacement convention
(Λ_ij = λ_e when the edge carries a DOF, else λ⁰ + u_i + u_j); spherical_gradient
Pass 1 now uses it, and the edge gradient is α_opp⁺ + α_opp⁻ − θ_e (θ_e default
π), exactly matching Java. The energy is the path integral of this gradient, so it
follows automatically. The vertex-only path is bit-for-bit unchanged (no edge
DOFs → identical to before). All 237 tests pass.
4. ✅ FIXED — Spherical Hessian edge-DOF guard added
spherical_hessian.hpp builds only the vertex block; Java's spherical
conformalHessian includes edge terms. Same limitation as the Euclidean Hessian
but previously without the edge-DOF guard added in Finding 2.
Fix: added an always-compiled throw std::logic_error guard at the top of
spherical_hessian (mirrors Finding 2), so any attempt to use edge DOFs with the
spherical Hessian fails loudly instead of silently building a singular matrix.
All 237 tests pass.
5. ℹ️ NOTE — CP-Euclidean energy uses clausen2, Java uses clausen
CPEuclideanFunctional.java:226 calls Clausen.clausen (the BORDERLINE-2.0944
polynomial); the C++ energy calls clausen2 (the Chebyshev variant). Both compute
the same Cl₂ integral to ~machine precision, and the term appears only in the
energy (not the Newton-driving gradient). No functional impact. The C++ never
ported the clausen() variant at all — harmless.
6. ✅ FIXED — Period-matrix reduction now uses the faithful Java port
Two reductions coexist:
discrete_elliptic_utility.hpp::normalizeModulusis a faithful line-by-line port of JavaDiscreteEllipticUtility.normalizeModulus— it folds τ into0 ≤ Re(τ) ≤ ½,Im(τ) ≥ 0,|τ| ≥ 1(the extraRe ≥ 0fold uses the mirror symmetry τ ≅ −τ̄).period_matrix.hpp::reduce_to_fundamental_domainreduces to the standard SL(2,ℤ) domain−½ ≤ Re(τ) < ½,|τ| ≥ 1(noRe ≥ 0fold).
compute_period_matrix (the production path, and the one the torus_8x8 τ test
exercises) calls reduce_to_fundamental_domain, so normalizeModulus is
never used outside its own unit test (verified by grep). Consequence: the C++
production τ can land with Re(τ) < 0 where Java would report the mirrored
+|Re(τ)|. Same conformal type up to orientation, but not equal to the Java
oracle value — this will bite any golden-value cross-check (missing-test item 5).
Fix (option a): compute_period_matrix now calls normalizeModulus, so the
production τ matches the Java oracle exactly (folds into 0 ≤ Re ≤ ½, Im ≥ 0,
|τ| ≥ 1 via the mirror symmetry). normalizeModulus is no longer dead code. The
existing torus_8x8 τ test still passes (Re ∈ [0,½] ⊂ [−½,½] so it satisfies
is_in_fundamental_domain too). reduce_to_fundamental_domain is retained for
callers that explicitly want the canonical (non-mirror-folded) SL(2,ℤ) domain.
7. ℹ️ NOTE — Cut graph is tree-cotree, not a canonical homology basis
cut_graph.hpp (compute_cut_graph) runs the Erickson–Whittlesey tree-cotree
algorithm: primal BFS tree, dual BFS cotree, the remaining 2g edges generate
H₁. Java's CanonicalBasisUtility.getCanonicalHomologyBasis goes further — it
builds the intersection form on the generators and solves for a canonical
symplectic basis {a₁..a_g, b₁..b_g} (and uses weighted shortest-path
cycles, not unweighted BFS).
- Genus 1: harmless — there is only one generator pair, and
compute_period_matrixreduces τ to the fundamental domain, so the basis choice washes out. The common case matches. - Genus > 1: the C++
omega[]ordering is arbitrary (edge-iteration order), soτ = ω₂/ω₁is not the canonical Riemann period matrix. This is already flagged in theperiod_matrix.hppheader ("Computing Ω from holonomy data requires integration of holomorphic differentials — not implemented") so it is consistent with the documented contract, not a regression.
8. ℹ️ NOTE — Holonomy translation assumes zero residual rotation
detail::euclidean_holonomy (layout.hpp) develops each face along a dual tree
that never crosses a cut edge, then reads each generator's translation as the
midpoint displacement ω = midA − midB of the shared cut edge between its two
independent developments. This equals the true deck-translation only when the
two developments differ by a pure translation (linear part = identity), i.e. for
a perfectly flat cone metric (Θ ≡ 2π). When Newton has not fully converged (small
residual cone-angle defect), there is a residual rotation and the midpoint
displacement is a first-order approximation rather than the exact ω. A fully
robust version would fit the rigid motion mapping edge (Bs,Bt)↦(At,As) and read
its translation part. Acceptable for converged flat tori (the torus_8x8 test
passes); worth tightening if higher-genus or under-converged inputs are used.
Findings in non-ported (conformallab++-only) math modules
These modules have no Java reference — they were ported from the literature (Luo 2004, Glickenstein 2011, Bowers–Stephenson 2004) or are original additions. They were audited against the cited formulas / first principles rather than Java.
9. ✅ FIXED — Inversive-distance gradient now uses limiting angles (consistent with Finding 1)
inversive_distance_functional.hpp::inversive_distance_gradient previously detected
a triangle-inequality-violating face two ways and skipped it in both:
if (l12sq <= 0 || l23sq <= 0 || l31sq <= 0) continue;(non-real circle config)if (!fa.valid) continue;(degenerate-but-real triangle)
The second skip was the same situation Finding 1 fixed for Euclidean/Spherical:
euclidean_angles returns the limiting angles (π opposite the over-long edge,
0/0 for the others) with valid=false, but this code threw them away.
Fix: removed the if (!fa.valid) continue; skip so the limiting angles flow
into the gradient (the BPS-style convex C¹ extension), mirroring Finding 1; added a
comment explaining the rationale. The first skip (l*sq <= 0, a genuinely non-real
circle configuration with no limiting angle) is kept. All 237 tests pass.
10. ℹ️ NOTE — Inversive-distance edge length has no overflow centring
id_detail::edge_length_squared computes ℓ² = rᵢ² + rⱼ² + 2·I·rᵢ·rⱼ with
r = exp(u) directly — no log-centring like euclidean_angles's μ. The angle
computation is still safe (it re-centres internally on log ℓ²), but the raw ℓ²
can overflow for extreme |u|. Cosmetic robustness only; not hit in practice.
11. ℹ️ NOTE — Verified correct (no action)
Audited against the literature and found faithful:
trilaterate_2d / _sph / _hyp(layout.hpp) — circle-circle intersection (ℝ²), spherical law of cosines viaα·pa+β·pb+γ·(pa×pb), and hyperbolic law of cosines + Poincaré-disk Möbius placement. All three exact; left/CCW side correct.newton_solver.hpp— meritf = ½‖G‖²; Newton dir is always a descent dir (∇f·dx = −‖G‖²), Armijo tests algebraically correct in both phases; spherical solver factorises−H(PSD) andd_sd = −H·G = −∇fuses the un-negated H — all consistent. SparseQR fallback handles the gauge null space (valid because Gauss–Bonnet makesG ⟂ 𝟙).gauss_bonnet.hpp—χ=V−E+F,g=(2−χ)/2,enforceshiftδ=(lhs−rhs)/Vmakes the new sum equal2π·χexactly.- Normalisations — Euclidean PCA, spherical Rodrigues (mean→north), hyperbolic weighted Fréchet-mean Möbius centring: all correct (spherical has a benign antipodal-mean edge case that no-ops).
Missing test cases (gaps observed during the audit)
-
Degenerate / flipped-triangle gradient — no test drives a face through the triangle-inequality boundary to lock in Finding 1. Add: build a triangle with one edge length > sum of the others, assert the gradient picks up the π corner (not 0). Cover both Euclidean and Spherical.
-
euclidean_hessianedge-DOF guard — no test asserts the newthrowfires whenassign_euclidean_all_dof_indicesis used. Add anEXPECT_THROW. -
✅ DONE (2026-05-29) — Holonomy / period matrix end-to-end now covers three tori of revolution (
HolonomyEndToEnd.{Torus4x4,TorusHex6x6,Torus8x8}), each asserting τ against the analytic modulusi·√(R²−r²)/randRe(τ) ≈ 0. (This also fixed the Euclidean holonomy extraction — develop across the dual tree only — seelayout.hpp/cut_graph.hpp.) -
⚠️ PARTLY DONE (2026-05-29) — added
EdgeGradient_RegularTetClosedForm: an independent closed-form oracle that pins each edge-DOF gradient toπ/3(= 2·(2π/3) − π) on the regular spherical tetrahedron, where the corner angle 2π/3 follows from the spherical law of cosines. This locks the Finding-3 formulaα_opp⁺ + α_opp⁻ − θ_eagainst a value the path-integral FD check cannot detect (the energy is the integral of G, so FD only proves curl-freeness). Still open: (i) Newton-to-convergence with edge DOFs is blocked by the Finding-4 spherical-Hessian guard (throwon edge DOFs), so a converged-metric test needs an FD-Hessian or guard relaxation first; (ii) a live-Java golden-value oracle still requires running the upstream library. -
Cross-check vs Java numeric oracles — there is no test that pins C++ functional/gradient values against recorded Java outputs on a fixed mesh. A handful of golden-value tests (energy + gradient at a known
x) would catch any future silent divergence from the reference far more directly than the self-consistent FD checks (which only verify curl-freeness, since the C++ energy is itself the path-integral of its own gradient). -
CP-Euclidean
clausenvsclausen2— a single test asserting|clausen(x) − clausen2(x)| < 1e-12across a sweep would document Finding 5 and guard against an approximation regression. -
Period-matrix domain convention (Finding 6) — production now uses
normalizeModulus(foldsRe ≥ 0). Add a torus whose true τ hasRe(τ) < 0before reduction and assert the resultingRe(τ) ≥ 0convention, so any future switch back toreduce_to_fundamental_domainis caught. Pair with a golden τ recorded from the JavaDiscreteEllipticUtilityon the same mesh. -
normalizeModulusvs Java oracle (Finding 6) — the faithfulnormalizeModulusis only checked against ad-hoc values. Add a few recorded JavanormalizeModulusoutputs as golden values to lock theRe ≥ 0fold. -
Higher-genus cut graph (Finding 7) — only genus-1 is exercised end-to-end. Add a genus-2 mesh and assert
compute_cut_graphreturns exactly2g = 4cut edges andgenus == 2, documenting that the basis is not canonical (so no τ correctness is claimed there). -
Inversive-distance degenerate face (Finding 9) — no test drives an inversive-distance triangle through the triangle-inequality boundary. Now that Finding 9 mirrors Finding 1 (limiting angles used, not skipped), add the analogue of missing-test item 1: assert the corner opposite the over-long edge is picked up as π rather than silently skipped.