From 46c0b63de82e2505b3f6ce922163c9c3062d7eea Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Sun, 31 May 2026 00:07:07 +0200 Subject: [PATCH] =?UTF-8?q?fix(gauss-bonnet):=20delete=20HyperIdeal=20over?= =?UTF-8?q?loads=20=E2=80=94=20wrong=20identity=20for=20hyperbolic=20metri?= =?UTF-8?q?cs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- code/include/gauss_bonnet.hpp | 55 +++++++++++++---- code/tests/cgal/test_phase6.cpp | 72 +++++++++++++++++++++++ doc/reviewer/external-audit-2026-05-30.md | 33 +++++++++-- 3 files changed, 142 insertions(+), 18 deletions(-) diff --git a/code/include/gauss_bonnet.hpp b/code/include/gauss_bonnet.hpp index 1d92c68..8ac316f 100644 --- a/code/include/gauss_bonnet.hpp +++ b/code/include/gauss_bonnet.hpp @@ -7,14 +7,28 @@ // Phase 6 — Gauss–Bonnet consistency check for prescribed target angles. // // Before calling newton_*() with custom target angles, verify that -// the angle defect sum matches the topology: +// the angle defect sum matches the topology. // -// Σ_v (2π − Θ_v) = 2π · χ(M) (Euclidean / flat) -// Σ_v (2π − Θ_v) > 0 (spherical, χ > 0) -// Σ_v (2π − Θ_v) < 0 (hyperbolic, χ < 0) +// ┌─────────────────────────────────────────────────────────────────────────┐ +// │ Geometry Identity to satisfy │ +// │ ───────────────────────────────────────────────────────────────────── │ +// │ Euclidean/flat Σ_v (2π − Θ_v) = 2π · χ(M) (exact equality) │ +// │ Spherical Σ_v (2π − Θ_v) > 0 (sufficient, χ > 0) │ +// │ │ +// │ HyperIdeal — NOT SUPPORTED by this header. │ +// │ The correct hyperbolic Gauss–Bonnet identity is │ +// │ Σ_v (2π − Θ_v) − Area(M) = 2π · χ(M) │ +// │ which differs from the Euclidean identity by the Area(M) > 0 term. │ +// │ Computing Area(M) from the HyperIdeal DOFs is non-trivial. │ +// │ gauss_bonnet_sum(mesh, HyperIdealMaps) and │ +// │ enforce_gauss_bonnet(mesh, HyperIdealMaps) are therefore DELETED. │ +// │ Do NOT call check_gauss_bonnet before newton_hyper_ideal — │ +// │ it is not needed; the HyperIdeal energy is strictly convex so Newton │ +// │ converges without a pre-check. │ +// └─────────────────────────────────────────────────────────────────────────┘ // -// If this fails, no conformal factor can realise the target angles and -// Newton will silently fail to converge. +// If the Euclidean/Spherical check fails, no conformal factor can realise +// the target angles and Newton will silently fail to converge. // // PRECONDITION — closed meshes only. Every function here sums (2π − Θ_v) // over ALL vertices. On a mesh with boundary the boundary vertices carry a @@ -26,11 +40,12 @@ // API: // int euler_characteristic(mesh) // int genus(mesh) -// double gauss_bonnet_sum(mesh, maps) — Σ(2π − Θ_v) +// double gauss_bonnet_sum(mesh, EuclideanMaps/SphericalMaps) — Σ(2π − Θ_v) // double gauss_bonnet_rhs(mesh) — 2π · χ(M) // double gauss_bonnet_deficit(mesh, maps) — lhs − rhs (0 = satisfied) // void check_gauss_bonnet(mesh, maps [, tol]) — throws if violated // void enforce_gauss_bonnet(mesh, maps) — shifts θ_v by uniform Δ +// (HyperIdealMaps overloads are deleted — see box above) #include "conformal_mesh.hpp" #include "euclidean_functional.hpp" @@ -78,15 +93,23 @@ inline double gauss_bonnet_sum( } /// `gauss_bonnet_sum` for the Euclidean-functional property bundle. -inline double gauss_bonnet_sum(const ConformalMesh& m, const EuclideanMaps& mp) +inline double gauss_bonnet_sum(const ConformalMesh& m, const EuclideanMaps& mp) { return gauss_bonnet_sum(m, mp.theta_v); } /// `gauss_bonnet_sum` for the Spherical-functional property bundle. -inline double gauss_bonnet_sum(const ConformalMesh& m, const SphericalMaps& mp) - { return gauss_bonnet_sum(m, mp.theta_v); } -/// `gauss_bonnet_sum` for the HyperIdeal-functional property bundle. -inline double gauss_bonnet_sum(const ConformalMesh& m, const HyperIdealMaps& mp) +inline double gauss_bonnet_sum(const ConformalMesh& m, const SphericalMaps& mp) { return gauss_bonnet_sum(m, mp.theta_v); } +// gauss_bonnet_sum for HyperIdealMaps is intentionally DELETED. +// The correct hyperbolic Gauss–Bonnet identity is +// Σ(2π−Θ_v) − Area(M) = 2π·χ(M) +// not the Euclidean form Σ(2π−Θ_v) = 2π·χ(M). Providing this overload +// would silently skip the Area term, making check_gauss_bonnet always +// fail for valid hyperbolic targets (e.g. a genus-2 mesh with Θ_v=2π +// gives Σ(2π−Θ_v)=0 but 2π·χ=−4π → deficit=4π ≠ 0 every time). +// Use newton_hyper_ideal directly — no pre-check is needed because the +// HyperIdeal energy is strictly convex (Springborn 2020 Theorem 1.3). +inline double gauss_bonnet_sum(const ConformalMesh&, const HyperIdealMaps&) = delete; + // ── Right-hand side 2π · χ(M) ─────────────────────────────────────────────── /// Right-hand side of Gauss-Bonnet: `2π · χ(M)`. @@ -157,10 +180,18 @@ inline void enforce_gauss_bonnet( } /// Distribute the Gauss-Bonnet deficit uniformly across `maps.theta_v`. +/// Supported for EuclideanMaps and SphericalMaps only. +/// HyperIdealMaps overload is deleted — see header comment for why. template inline void enforce_gauss_bonnet(ConformalMesh& mesh, Maps& maps) { enforce_gauss_bonnet(mesh, maps.theta_v); } +// enforce_gauss_bonnet for HyperIdealMaps is intentionally DELETED. +// The Euclidean identity Σ(2π−Θ_v)=2π·χ is not the correct pre-condition +// for HyperIdeal. Calling this function would silently shift Θ_v to +// satisfy the wrong identity, producing incorrect target angles. +inline void enforce_gauss_bonnet(ConformalMesh&, HyperIdealMaps&) = delete; + } // namespace conformallab diff --git a/code/tests/cgal/test_phase6.cpp b/code/tests/cgal/test_phase6.cpp index f77dfaf..f271207 100644 --- a/code/tests/cgal/test_phase6.cpp +++ b/code/tests/cgal/test_phase6.cpp @@ -137,6 +137,78 @@ TEST(GaussBonnet, ManuallySetAnalyticalTheta_PassesCheck) EXPECT_NO_THROW(check_gauss_bonnet(m, maps)); } +// ════════════════════════════════════════════════════════════════════════════ +// GaussBonnet — HyperIdeal API guard (Finding-B from external-audit-2026-05-30) +// +// gauss_bonnet_sum(mesh, HyperIdealMaps) and +// enforce_gauss_bonnet(mesh, HyperIdealMaps) are intentionally DELETED. +// +// Reason: the correct hyperbolic Gauss–Bonnet identity is +// Σ_v (2π − Θ_v) − Area(M) = 2π · χ(M) +// not the Euclidean form Σ(2π−Θ_v) = 2π·χ. For a regular (Θ_v=2π) genus-2 +// surface: Σ(2π−2π)=0 but 2π·χ=−4π, so the Euclidean check would always +// throw "deficit = 4π" for a perfectly valid HyperIdeal target. +// +// Compile-time enforcement: gauss_bonnet_sum / enforce_gauss_bonnet with +// HyperIdealMaps are = delete, so any accidental call is a compile error. +// The static_asserts below confirm this is wired correctly. +// +// The runtime test shows the discrepancy numerically: even for a regular +// tetrahedron where all Θ_v=2π (a valid all-hyper-ideal starting point), +// the "Euclidean sum" is 0 but the correct RHS for χ=2 is 4π — the +// Euclidean check would be off by 4π. +// ════════════════════════════════════════════════════════════════════════════ + +// Invocability check via SFINAE: tries to form the call expression in an +// unevaluated context; a deleted function causes substitution failure. +namespace { + template + auto try_gb_sum(int) -> decltype(gauss_bonnet_sum( + std::declval(), + std::declval()), std::true_type{}); + template + std::false_type try_gb_sum(...); +} // namespace + +static_assert(!decltype(try_gb_sum(0))::value, + "gauss_bonnet_sum must NOT be invocable with HyperIdealMaps"); +static_assert( decltype(try_gb_sum(0))::value, + "gauss_bonnet_sum must still be invocable with EuclideanMaps"); +static_assert( decltype(try_gb_sum(0))::value, + "gauss_bonnet_sum must still be invocable with SphericalMaps"); + +TEST(GaussBonnet, HyperIdeal_EuclideanSumDiscrepancy_DocumentsWhyCheckIsDeleted) +{ + // On a tetrahedron (χ=2) with all Θ_v = 2π: + // Euclidean sum Σ(2π−2π) = 0 + // but 2π·χ = 4π + // deficit (Euclidean formula) = 0 − 4π = −4π ← WRONG check for HyperIdeal + // + // The HyperIdeal identity is Σ(2π−Θ_v) − Area = 2π·χ. + // Area > 0 for any non-degenerate hyperbolic metric, so the real deficit + // would be much smaller. This test documents the mismatch numerically + // so any future re-introduction of the HyperIdeal overload is caught. + auto m = make_tetrahedron(); + auto hi_maps = setup_hyper_ideal_maps(m); + // All theta_v default to 2π (regular vertex target). + + // Access the raw property map directly (not via the deleted bundle overload) + // to compute the Euclidean-style sum — just for documentation purposes. + double euclid_sum = gauss_bonnet_sum(m, hi_maps.theta_v); // raw map: OK + double rhs = gauss_bonnet_rhs(m); // 2π·χ = 4π + + EXPECT_NEAR(euclid_sum, 0.0, 1e-12) // Σ(2π−2π) = 0 + << "Expected Euclidean sum = 0 for all-regular HyperIdeal targets"; + EXPECT_NEAR(rhs, 4.0 * M_PI, 1e-12) // 2π·χ(tetrahedron) = 4π + << "Expected RHS = 4π for tetrahedron (χ=2)"; + + // The Euclidean deficit would be 0 − 4π = −4π: completely wrong for HyperIdeal. + // If check_gauss_bonnet were called with HyperIdealMaps it would ALWAYS throw + // here, even though Θ_v=2π is a valid regular-vertex HyperIdeal target. + EXPECT_NEAR(euclid_sum - rhs, -4.0 * M_PI, 1e-10) + << "Euclidean deficit for HyperIdeal target = −4π: confirms the deleted API is correct"; +} + // ════════════════════════════════════════════════════════════════════════════ // CutGraph — tree-cotree algorithm // ════════════════════════════════════════════════════════════════════════════ diff --git a/doc/reviewer/external-audit-2026-05-30.md b/doc/reviewer/external-audit-2026-05-30.md index 5ed628e..b76245c 100644 --- a/doc/reviewer/external-audit-2026-05-30.md +++ b/doc/reviewer/external-audit-2026-05-30.md @@ -247,12 +247,33 @@ being instantiated with `HyperIdealMaps`. Add a comment explaining why. compares against `2π·χ`. This requires computing the area from the HyperIdeal metric, which is non-trivial. +### Resolution (2026-05-31) + +1. **`gauss_bonnet_sum(mesh, HyperIdealMaps)` deleted** — replaced with + `= delete` overload and a multi-line comment explaining the Area-term + discrepancy. Attempting to call this is now a compile error. +2. **`enforce_gauss_bonnet(mesh, HyperIdealMaps&)` deleted** — explicit + `= delete` overload prevents the generic template from being silently + instantiated with HyperIdealMaps. +3. **Header comment block rewritten** — now has a clear box explaining + which geometries are supported (Euclidean/Spherical) and which are not + (HyperIdeal), with the correct hyperbolic Gauss–Bonnet identity shown. +4. **One new GTest** in `test_phase6.cpp`: + - `HyperIdeal_EuclideanSumDiscrepancy_DocumentsWhyCheckIsDeleted` — + verifies numerically that for a regular (Θ_v=2π) tetrahedron the + Euclidean sum = 0 while 2π·χ = 4π, i.e. the Euclidean check would + produce deficit = −4π for a valid HyperIdeal target. +5. **Three compile-time `static_assert`s** in the test file (SFINAE-based) + confirm that `gauss_bonnet_sum` is NOT invocable with HyperIdealMaps + but IS invocable with EuclideanMaps and SphericalMaps. + +**Test result:** 263/263 CGAL tests pass. No regressions. + ### Acceptance criteria -- [ ] Calling `check_gauss_bonnet(mesh, hyper_ideal_maps)` on a valid genus-2 mesh - with Θ_v = 2π either (a) does not compile, (b) throws with a meaningful message - distinguishing "wrong formula" from "actual violation", or (c) is removed -- [ ] A comment explains why Euclidean G-B does not apply to HyperIdeal -- [ ] 246 CGAL tests still pass +- [x] Calling `check_gauss_bonnet(mesh, hyper_ideal_maps)` → compile error +- [x] Calling `enforce_gauss_bonnet(mesh, hyper_ideal_maps)` → compile error +- [x] Comments explain why Euclidean G-B does not apply to HyperIdeal +- [x] 263 CGAL tests pass, 0 failed --- @@ -679,7 +700,7 @@ index. Add a comment: | ID | File | Lines | Type | Severity | Status | |----|------|-------|------|----------|--------| | A | `hyper_ideal_functional.hpp` | 319–344 | Bug | Critical (mixed config) | ✅ Fixed 2026-05-30 | -| B | `gauss_bonnet.hpp` | 87–88, 128–134 | API error | Medium | 🟡 Open | +| B | `gauss_bonnet.hpp` | 87–88, 128–134 | API error | Medium | ✅ Fixed 2026-05-31 | | C | `euclidean_hessian.hpp` | 26–27, 57–58 | Doc error | Medium | 🟡 Open | | D | `euclidean_functional.hpp` + 2 others | 97–107 | Doc error | Medium | 🟡 Open | | E | `cp_euclidean_functional.hpp` | 338–349, 373–387 | Inconsistency | Medium | 🟡 Open |