fix(solver+io): B1 block-FD Hessian, V3 NaN/Inf guard, C1 ok-flag (audit quick-wins)
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>
This commit is contained in:
@@ -29,6 +29,7 @@
|
|||||||
#include <CGAL/boost/graph/helpers.h>
|
#include <CGAL/boost/graph/helpers.h>
|
||||||
#include <string>
|
#include <string>
|
||||||
#include <stdexcept>
|
#include <stdexcept>
|
||||||
|
#include <cmath>
|
||||||
|
|
||||||
namespace conformallab {
|
namespace conformallab {
|
||||||
|
|
||||||
@@ -62,6 +63,12 @@ inline ConformalMesh load_mesh(const std::string& filename)
|
|||||||
throw std::runtime_error(
|
throw std::runtime_error(
|
||||||
"conformallab: mesh from " + filename +
|
"conformallab: mesh from " + filename +
|
||||||
" is not triangulated (functionals require triangle faces)");
|
" is not triangulated (functionals require triangle faces)");
|
||||||
|
for (auto v : mesh.vertices()) {
|
||||||
|
const auto& p = mesh.point(v);
|
||||||
|
if (!std::isfinite(p.x()) || !std::isfinite(p.y()) || !std::isfinite(p.z()))
|
||||||
|
throw std::runtime_error(
|
||||||
|
"conformallab: non-finite vertex coordinate in " + filename);
|
||||||
|
}
|
||||||
return mesh;
|
return mesh;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -103,14 +103,18 @@ inline Eigen::VectorXd solve_with_fallback(
|
|||||||
/// Solve `A·x = rhs` with the same SimplicialLDLT → SparseQR fallback
|
/// Solve `A·x = rhs` with the same SimplicialLDLT → SparseQR fallback
|
||||||
/// strategy used inside all three Newton solvers. If `fallback_used`
|
/// strategy used inside all three Newton solvers. If `fallback_used`
|
||||||
/// is non-null, it is set to `true` iff the SparseQR fallback ran.
|
/// is non-null, it is set to `true` iff the SparseQR fallback ran.
|
||||||
|
/// If `ok` is non-null, it is set to `true` iff at least one solver succeeded.
|
||||||
/// Returns `Eigen::VectorXd::Zero(rhs.size())` if both solvers fail.
|
/// Returns `Eigen::VectorXd::Zero(rhs.size())` if both solvers fail.
|
||||||
inline Eigen::VectorXd solve_linear_system(
|
inline Eigen::VectorXd solve_linear_system(
|
||||||
const Eigen::SparseMatrix<double>& A,
|
const Eigen::SparseMatrix<double>& A,
|
||||||
const Eigen::VectorXd& rhs,
|
const Eigen::VectorXd& rhs,
|
||||||
bool* fallback_used = nullptr)
|
bool* fallback_used = nullptr,
|
||||||
|
bool* ok = nullptr)
|
||||||
{
|
{
|
||||||
bool ok = false;
|
bool ok_local = false;
|
||||||
return detail::solve_with_fallback(A, rhs, ok, fallback_used);
|
auto x = detail::solve_with_fallback(A, rhs, ok_local, fallback_used);
|
||||||
|
if (ok) *ok = ok_local;
|
||||||
|
return x;
|
||||||
}
|
}
|
||||||
|
|
||||||
namespace detail { // re-open for the remaining helpers
|
namespace detail { // re-open for the remaining helpers
|
||||||
@@ -429,8 +433,8 @@ inline NewtonResult newton_hyper_ideal(
|
|||||||
return res;
|
return res;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Hessian (numerical FD) + solve H·Δx = −G ─────────────────────────
|
// ── Hessian (block-FD, ~33–1166× faster than full-FD) + solve H·Δx = −G
|
||||||
auto H = hyper_ideal_hessian_sym(mesh, x, m, hess_eps);
|
auto H = hyper_ideal_hessian_block_fd_sym(mesh, x, m, hess_eps);
|
||||||
bool ok = false;
|
bool ok = false;
|
||||||
Eigen::VectorXd dx = detail::solve_with_fallback(H, -G, ok);
|
Eigen::VectorXd dx = detail::solve_with_fallback(H, -G, ok);
|
||||||
if (!ok) break;
|
if (!ok) break;
|
||||||
|
|||||||
@@ -18,6 +18,7 @@
|
|||||||
#include <cmath>
|
#include <cmath>
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
#include <filesystem>
|
#include <filesystem>
|
||||||
|
#include <fstream>
|
||||||
#include <stdexcept>
|
#include <stdexcept>
|
||||||
|
|
||||||
using namespace conformallab;
|
using namespace conformallab;
|
||||||
@@ -171,3 +172,42 @@ TEST(MeshIO, SaveLoadConvenienceWrappers)
|
|||||||
|
|
||||||
rm(path);
|
rm(path);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
// V3: load_mesh throws on non-finite vertex coordinate (NaN / Inf)
|
||||||
|
//
|
||||||
|
// A mesh file containing NaN or Inf vertex coordinates must be rejected at
|
||||||
|
// the I/O boundary — before the coordinates can silently poison the solver.
|
||||||
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
|
TEST(MeshIO, LoadMeshThrowsOnNaNCoordinate)
|
||||||
|
{
|
||||||
|
// Write a minimal OFF triangle with a NaN in the first vertex.
|
||||||
|
auto path = tmp_path("nan.off");
|
||||||
|
{
|
||||||
|
std::ofstream ofs(path);
|
||||||
|
ofs << "OFF\n3 1 0\n"
|
||||||
|
<< "nan 0.0 0.0\n"
|
||||||
|
<< "1.0 0.0 0.0\n"
|
||||||
|
<< "0.0 1.0 0.0\n"
|
||||||
|
<< "3 0 1 2\n";
|
||||||
|
}
|
||||||
|
EXPECT_THROW(load_mesh(path), std::runtime_error);
|
||||||
|
rm(path);
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST(MeshIO, LoadMeshThrowsOnInfCoordinate)
|
||||||
|
{
|
||||||
|
// Write a minimal OFF triangle with Inf in the y-coordinate.
|
||||||
|
auto path = tmp_path("inf.off");
|
||||||
|
{
|
||||||
|
std::ofstream ofs(path);
|
||||||
|
ofs << "OFF\n3 1 0\n"
|
||||||
|
<< "0.0 0.0 0.0\n"
|
||||||
|
<< "1.0 inf 0.0\n"
|
||||||
|
<< "0.0 1.0 0.0\n"
|
||||||
|
<< "3 0 1 2\n";
|
||||||
|
}
|
||||||
|
EXPECT_THROW(load_mesh(path), std::runtime_error);
|
||||||
|
rm(path);
|
||||||
|
}
|
||||||
|
|||||||
@@ -501,3 +501,81 @@ TEST(SparseQRFallback, Euclidean_ClosedMeshNoPinConverges)
|
|||||||
"grad_inf_norm = " << res.grad_inf_norm;
|
"grad_inf_norm = " << res.grad_inf_norm;
|
||||||
EXPECT_LT(res.grad_inf_norm, 1e-8);
|
EXPECT_LT(res.grad_inf_norm, 1e-8);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
// C1: solve_linear_system exposes the ok flag via the new out-parameter
|
||||||
|
//
|
||||||
|
// The C1 audit finding was that solve_linear_system silently dropped the
|
||||||
|
// internal ok flag, making double-solver failure invisible to callers.
|
||||||
|
// These tests verify that the new optional bool* ok parameter is correctly
|
||||||
|
// wired: ok=true for successful solves, and the parameter is optional (null
|
||||||
|
// is safe).
|
||||||
|
//
|
||||||
|
// Note: constructing a matrix where Eigen's SparseQR hard-fails is not
|
||||||
|
// practical — SparseQR reports Eigen::Success even for rank-0 inputs,
|
||||||
|
// returning a zero min-norm solution. The observable contract for callers is
|
||||||
|
// therefore: ok=true whenever a solver reports success; ok=false only when
|
||||||
|
// both report failure (an edge case Eigen essentially never produces). The
|
||||||
|
// fix in C1 ensures that if that edge case ever fires it propagates to the
|
||||||
|
// caller — previously it was silently swallowed.
|
||||||
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
|
TEST(SparseQRFallback, OkFlag_TrueOnFullRankSystem)
|
||||||
|
{
|
||||||
|
// 3×3 diagonal PD matrix: LDLT succeeds → ok must be true.
|
||||||
|
Eigen::SparseMatrix<double> A(3, 3);
|
||||||
|
A.insert(0, 0) = 1.0;
|
||||||
|
A.insert(1, 1) = 2.0;
|
||||||
|
A.insert(2, 2) = 3.0;
|
||||||
|
A.makeCompressed();
|
||||||
|
|
||||||
|
Eigen::VectorXd rhs(3);
|
||||||
|
rhs << 1.0, 4.0, 9.0;
|
||||||
|
|
||||||
|
bool fallback = true;
|
||||||
|
bool ok = false;
|
||||||
|
Eigen::VectorXd x = conformallab::solve_linear_system(A, rhs, &fallback, &ok);
|
||||||
|
|
||||||
|
EXPECT_TRUE(ok) << "Full-rank system: ok must be true";
|
||||||
|
EXPECT_FALSE(fallback) << "Full-rank system: LDLT path, no fallback expected";
|
||||||
|
EXPECT_NEAR(x[0], 1.0, 1e-12);
|
||||||
|
EXPECT_NEAR(x[1], 2.0, 1e-12);
|
||||||
|
EXPECT_NEAR(x[2], 3.0, 1e-12);
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST(SparseQRFallback, OkFlag_TrueOnSingularSystemViaFallback)
|
||||||
|
{
|
||||||
|
// Singular matrix (zero row/col 1) — LDLT fails, SparseQR succeeds → ok=true.
|
||||||
|
Eigen::SparseMatrix<double> A(3, 3);
|
||||||
|
A.insert(0, 0) = 2.0;
|
||||||
|
A.insert(2, 2) = 3.0;
|
||||||
|
A.makeCompressed();
|
||||||
|
|
||||||
|
Eigen::VectorXd rhs(3);
|
||||||
|
rhs << 2.0, 0.0, 3.0;
|
||||||
|
|
||||||
|
bool fallback = false;
|
||||||
|
bool ok = false;
|
||||||
|
Eigen::VectorXd x = conformallab::solve_linear_system(A, rhs, &fallback, &ok);
|
||||||
|
|
||||||
|
EXPECT_TRUE(ok) << "Singular-but-consistent system: SparseQR succeeds → ok must be true";
|
||||||
|
EXPECT_TRUE(fallback) << "Singular system: fallback must have been triggered";
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST(SparseQRFallback, OkFlag_NullPointerIsSafe)
|
||||||
|
{
|
||||||
|
// Calling with ok=nullptr must not crash (backward-compatibility).
|
||||||
|
Eigen::SparseMatrix<double> A(2, 2);
|
||||||
|
A.insert(0, 0) = 1.0;
|
||||||
|
A.insert(1, 1) = 1.0;
|
||||||
|
A.makeCompressed();
|
||||||
|
|
||||||
|
Eigen::VectorXd rhs(2);
|
||||||
|
rhs << 3.0, 5.0;
|
||||||
|
|
||||||
|
EXPECT_NO_THROW({
|
||||||
|
Eigen::VectorXd x = conformallab::solve_linear_system(A, rhs, nullptr, nullptr);
|
||||||
|
EXPECT_NEAR(x[0], 3.0, 1e-12);
|
||||||
|
EXPECT_NEAR(x[1], 5.0, 1e-12);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user