conformallab/ConformalLabpp
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(dof-assign): correct "pin before" docs + add gauge-vertex overloads

Browse Source 
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>
This commit is contained in:
Tarik Moussa
2026-05-31 00:20:43 +02:00
parent bef5a0ceb7
commit cfbbc1b21f
5 changed files with 148 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
code/include/euclidean_functional.hpp
View File

@@ -95,10 +95,14 @@ inline EuclideanMaps setup_euclidean_maps(ConformalMesh& mesh)
/// Assign sequential DOF indices `0..n-1` to all vertices.
///
/// **Note:** does NOT pin a gauge vertex. For closed meshes the caller
/// must set one `m.v_idx[v] = -1` either before or after this call to
/// remove the rotational mode (the Newton solver's SparseQR fallback
/// will otherwise pick a minimum-norm solution but at higher cost).
/// **Note:** this overload assigns indices to ALL vertices unconditionally.
/// Any `v_idx` set before the call is overwritten. To pin a gauge vertex,
/// either use the two-argument overload below, or set `m.v_idx[v] = -1`
/// **after** this call. Pinning before this call has no effect.
///
/// For closed meshes one gauge vertex should be pinned to remove the
/// rotational null mode (the Newton solver's SparseQR fallback handles an
/// unpinned closed mesh automatically, but at higher cost).
inline int assign_euclidean_vertex_dof_indices(ConformalMesh& mesh, EuclideanMaps& m)
{
int idx = 0;
@@ -106,6 +110,20 @@ inline int assign_euclidean_vertex_dof_indices(ConformalMesh& mesh, EuclideanMap
return idx;
}
/// Assign sequential DOF indices to all vertices, pinning `gauge`
/// (`m.v_idx[gauge] = -1`). Use this overload on closed meshes to fix
/// the rotational gauge mode in a single call.
///
/// \returns The number of free DOFs assigned (`num_vertices − 1`).
inline int assign_euclidean_vertex_dof_indices(ConformalMesh& mesh, EuclideanMaps& m,
Vertex_index gauge)
{
int idx = 0;
for (auto v : mesh.vertices())
m.v_idx[v] = (v == gauge) ? -1 : idx++;
return idx;
}
/// Assign DOF indices for all vertices AND all edges (vertex-DOFs first,
/// then edge-DOFs). Use this overload for the "cyclic" formulation that
/// includes per-edge log-length DOFs (`λ_e`) on top of per-vertex scale

24
code/include/inversive_distance_functional.hpp
View File

@@ -122,11 +122,10 @@ inline InversiveDistanceMaps setup_inversive_distance_maps(ConformalMesh& mesh)
/// Assign sequential DOF indices `0..n-1` to every vertex.
///
/// **Note:** this overload does NOT pin a gauge vertex. The caller
/// is expected to either:
/// 1. set one `m.v_idx[v] = -1` *before* calling this function (then
/// the call is a no-op for that vertex) — OR —
/// 2. flip one assigned index back to `-1` *after* this function.
/// **Note:** this overload assigns indices to ALL vertices unconditionally.
/// Any `v_idx` set before the call is overwritten. To pin a gauge vertex,
/// either use the two-argument overload below, or set `m.v_idx[v] = -1`
/// **after** this call. Pinning before this call has no effect.
///
/// For a closed mesh, exactly one pin is required to remove the
/// global rotational mode.
@@ -138,6 +137,21 @@ inline int assign_inversive_distance_vertex_dof_indices(ConformalMesh& m
return idx;
}
/// Assign sequential DOF indices to all vertices, pinning `gauge`
/// (`m.v_idx[gauge] = -1`). Use this overload on closed meshes to fix
/// the rotational gauge mode in a single call.
///
/// \returns The number of free DOFs assigned (`num_vertices − 1`).
inline int assign_inversive_distance_vertex_dof_indices(ConformalMesh& mesh,
InversiveDistanceMaps& m,
Vertex_index gauge)
{
int idx = 0;
for (auto v : mesh.vertices())
m.v_idx[v] = (v == gauge) ? -1 : idx++;
return idx;
}
/// Count the free DOFs (vertices with `v_idx >= 0`).
inline int inversive_distance_dimension(const ConformalMesh& mesh,
const InversiveDistanceMaps& m)

23
code/include/spherical_functional.hpp
View File

@@ -87,7 +87,14 @@ inline SphericalMaps setup_spherical_maps(ConformalMesh& mesh)
}
/// Assign sequential DOF indices `0..n-1` to all vertices (no edge DOFs).
/// Caller is expected to pin one gauge vertex with `m.v_idx[v] = -1`.
///
/// **Note:** this overload assigns indices to ALL vertices unconditionally.
/// Any `v_idx` set before the call is overwritten. To pin a gauge vertex,
/// either use the two-argument overload below, or set `m.v_idx[v] = -1`
/// **after** this call. Pinning before this call has no effect.
///
/// On closed spherical surfaces exactly one gauge vertex must be pinned
/// to remove the global-scale null mode.
inline int assign_vertex_dof_indices(ConformalMesh& mesh, SphericalMaps& m)
{
int idx = 0;
@@ -95,6 +102,20 @@ inline int assign_vertex_dof_indices(ConformalMesh& mesh, SphericalMaps& m)
return idx;
}
/// Assign sequential DOF indices to all vertices, pinning `gauge`
/// (`m.v_idx[gauge] = -1`). Use this overload on closed spherical
/// surfaces to fix the global-scale gauge mode in a single call.
///
/// \returns The number of free DOFs assigned (`num_vertices − 1`).
inline int assign_vertex_dof_indices(ConformalMesh& mesh, SphericalMaps& m,
Vertex_index gauge)
{
int idx = 0;
for (auto v : mesh.vertices())
m.v_idx[v] = (v == gauge) ? -1 : idx++;
return idx;
}
/// Assign DOF indices for all vertices AND all edges (vertex-DOFs first,
/// then edge-DOFs). Mirrors `assign_euclidean_all_dof_indices` for the
/// cyclic spherical formulation.

84
code/tests/cgal/test_euclidean_functional.cpp
View File

@@ -645,3 +645,87 @@ TEST(EuclideanFunctional, CyclicHessian_Analytic_MatchesBlockFD_Tetrahedron)
max_sym = std::max(max_sym, std::abs(Ha.coeff(i, j) - Ha.coeff(j, i)));
EXPECT_LT(max_sym, 1e-9) << "analytic cyclic Hessian is not symmetric";
}
// ════════════════════════════════════════════════════════════════════════════
// DOF assignment gauge-vertex overload (Finding-D, external-audit-2026-05-30)
//
// The single-argument assign_euclidean_vertex_dof_indices() overwrites ALL
// v_idx unconditionally — setting a pin *before* the call has no effect.
// The two-argument overload (gauge vertex) pins the requested vertex in a
// single pass. These tests verify:
// (a) single-argument: gauge must be set AFTER, not before.
// (b) two-argument: the gauge vertex gets v_idx = -1, others are sequential.
// (c) Newton converges correctly when the gauge overload is used.
// ════════════════════════════════════════════════════════════════════════════
TEST(EuclideanDOFAssignment, SingleArg_PinBeforeHasNoEffect)
{
// Pin first vertex before the call → the call overwrites it → not pinned.
auto mesh = make_tetrahedron();
auto maps = setup_euclidean_maps(mesh);
auto first = *mesh.vertices().begin();
maps.v_idx[first] = -1; // set pin BEFORE — should have no effect
assign_euclidean_vertex_dof_indices(mesh, maps);
EXPECT_GE(maps.v_idx[first], 0)
<< "Pre-call pin was overwritten: v_idx[first] must be >= 0 after the call";
EXPECT_EQ(euclidean_dimension(mesh, maps),
static_cast<int>(mesh.number_of_vertices()))
<< "All vertices should be free after single-arg assign";
}
TEST(EuclideanDOFAssignment, TwoArg_GaugeIsPinnedOthersAreSequential)
{
auto mesh = make_tetrahedron();
auto maps = setup_euclidean_maps(mesh);
auto first = *mesh.vertices().begin();
int n = assign_euclidean_vertex_dof_indices(mesh, maps, first);
EXPECT_EQ(maps.v_idx[first], -1)
<< "Gauge vertex must have v_idx = -1";
EXPECT_EQ(n, static_cast<int>(mesh.number_of_vertices()) - 1)
<< "Returned DOF count must be num_vertices - 1";
EXPECT_EQ(euclidean_dimension(mesh, maps), n)
<< "euclidean_dimension must equal returned count";
// All non-gauge vertices must have distinct indices in [0, n).
std::vector<int> seen;
for (auto v : mesh.vertices()) {
int iv = maps.v_idx[v];
if (v == first) continue;
EXPECT_GE(iv, 0);
EXPECT_LT(iv, n);
seen.push_back(iv);
}
std::sort(seen.begin(), seen.end());
for (int i = 0; i < n; ++i)
EXPECT_EQ(seen[static_cast<std::size_t>(i)], i)
<< "DOF indices must be sequential 0..n-1";
}
TEST(EuclideanDOFAssignment, TwoArg_NewtonConvergesWithGaugeOverload)
{
// End-to-end: use the gauge overload, then run Newton — confirms the
// pinned-vertex DOF layout is consistent with the solver.
auto mesh = make_tetrahedron();
auto maps = setup_euclidean_maps(mesh);
compute_euclidean_lambda0_from_mesh(mesh, maps);
auto first = *mesh.vertices().begin();
int n = assign_euclidean_vertex_dof_indices(mesh, maps, first);
// Natural-theta: set targets = actual angle sums at x=0 so x*=0 is the solution.
std::vector<double> x0(static_cast<std::size_t>(n), 0.0);
auto G0 = euclidean_gradient(mesh, x0, maps);
for (auto v : mesh.vertices()) {
int iv = maps.v_idx[v];
if (iv >= 0) maps.theta_v[v] -= G0[static_cast<std::size_t>(iv)];
}
auto res = newton_euclidean(mesh, x0, maps, 1e-10, 50);
EXPECT_TRUE(res.converged)
<< "Newton did not converge with gauge-overload DOF assignment";
EXPECT_LT(res.grad_inf_norm, 1e-9);
}

2
doc/reviewer/external-audit-2026-05-30.md
View File

@@ -702,7 +702,7 @@ index. Add a comment:
| 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 | ✅ Fixed 2026-05-31 |
| C | `euclidean_hessian.hpp` | 26–27, 57–58 | Doc error | Medium | ✅ Fixed 2026-05-31 |
| D | `euclidean_functional.hpp` + 2 others | 97–107 | Doc error | Medium | 🟡 Open |
| D | `euclidean_functional.hpp` + 2 others | 97–107 | Doc error | Medium | ✅ Fixed 2026-05-31 |
| E | `cp_euclidean_functional.hpp` | 338–349, 373–387 | Inconsistency | Medium | 🟡 Open |
| F | test files | — | Test gap | Medium | 🟠 Open |
| G | test files | — | Test gap | Medium | 🟠 Open |