Skip to content

Add TriangleSoA with SIMD-ificiation of LinearBVH#88

Open
rmrsk wants to merge 123 commits into
mainfrom
perf
Open

Add TriangleSoA with SIMD-ificiation of LinearBVH#88
rmrsk wants to merge 123 commits into
mainfrom
perf

Conversation

@rmrsk

@rmrsk rmrsk commented Jul 2, 2026

Copy link
Copy Markdown
Owner

No description provided.

@rmrsk rmrsk self-assigned this Jul 3, 2026
rmrsk and others added 20 commits July 3, 2026 09:20
…docs

- Fix noexcept correctness bugs: Vec3T::operator[], unit(), lessLX() and
  MeshT::incrementWarning() all called throwing operations inside noexcept
  bodies; replaced map::at() with operator[] and .at() with EBGEOMETRY_EXPECT.
- Remove noexcept from ~54 allocating functions across Parser, Transform, CSG,
  BVH, DCEL (Face, Vertex, Mesh), MeshDistanceFunctions, and ImplicitFunction.
  Declaring noexcept on functions that call make_shared/emplace_back silently
  converts std::bad_alloc into std::terminate.
- Add noexcept where it was missing: Vec2T constructors, EdgeT constructors (x3),
  EdgeIteratorT constructors (x2), and five Transform virtual destructors that
  were inconsistent with their siblings.
- Add [[nodiscard]] to all 18 Parser read* functions; discarding the returned
  shared_ptr is always a programming error.
- Add EBGEOMETRY_EXPECT guards in Vec3T::minDir and Vec3T::maxDir.
- Add reserve() before emplace_back loops in FaceT gather functions and
  MeshT::getAllVertexCoordinates.
- Fix sign-compare warning in MollifyIF (int -> size_t loop variables).
- Fix wrong Doxygen @return on both MeshT::signedDistance overloads (said
  "negative inside"; DCEL inward-normal convention is positive inside).
- Add CMakePresets.json with debug/debug-san/release/release-test presets;
  update CI to use cmake --preset debug; add sanitizer and examples support
  to CMakeLists.txt and Tests/CMakeLists.txt.
- Add Examples/CMakeLists.txt so examples are buildable and testable via CTest
  with label filtering (unit vs example).
- Rewrite README quickstart; expand Contributing.rst with preset reference table
  and manual-options fallback for CMake < 3.22.
- Add TODO.md tracking all findings; one open item remains (sanityCheck return
  type, deferred as API-breaking).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UJSdT1HUkhxMw4DFYPZE3y
All warnings were caused by stale or mis-scoped literalinclude line ranges:

- ImplemBVH.rst: bottomUpSortAndPartition literalinclude pointed to the
  TreeBVH class declaration (lines 298-309, which started mid-Doxygen-comment)
  instead of the function declaration (lines 363-372).

- ImplemCSG.rst: Transform literalinclude started at line 20 (inside the
  first /** comment, missing the opening /**) and ended at line 111
  (mid-comment before Elongate); fixed to lines 19-130 (/** through Reflect).
  CSG literalinclude started at line 23 (same issue) and ended at line 153
  (mid-SmoothIntersection comment); fixed to lines 22-214 (through FiniteRepetition).
  Handwritten approximateBoundingVolumeOctree code-block had stale `noexcept`
  and a typo (`const T>&`); replaced with a literalinclude of the actual
  declaration (ImplicitFunction.hpp lines 57-75).

- ImplemOctree.rst: Both non-contiguous line selections pulled orphaned @param
  lines from the middles of different Doxygen comment blocks, producing garbled
  output. Replaced with coherent contiguous ranges that include full comment
  blocks: lines 75-100,180-200 for Construction; 75-76,102-119,202-214 for
  Traversal.

- Parsers.rst: All five literalinclude blocks had stale line numbers that
  pointed to the wrong functions (each block was shifted by roughly 14-20
  lines from a prior Parser refactor). Updated to show the correct functions:
  readIntoDCEL (137-157), readIntoMesh (159-179), readIntoFullBVH (181-211),
  readIntoCompactBVH (213-235), readIntoTriangleBVH (237-267).

- index.rst: Move Contributing.rst out of the Introduction toctree into its
  own top-level "Contributing and testing" section placed after Examples and
  before References. Add CSS rule to suppress the section heading in HTML view.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UJSdT1HUkhxMw4DFYPZE3y
…h std::clamp

Sign convention: flip DCEL face normals from inward to outward ((x2-x0)×(x2-x1)
instead of (x2-x1)×(x2-x0)), making signedDistance negative inside / positive
outside — matching the analytic SDF convention.  Same flip applied to Triangle
and all four TriangleSoA SIMD paths (SSE4.1 float, AVX float, AVX double W=4/8)
plus the scalar fallback; the point-in-triangle test now uses abs(s0+s1+s2)>=2
so it is correct for both inward and outward normals.  Updated Doxygen @return
strings in DCEL_Mesh.hpp and Contributing.rst note.  Tests flipped accordingly;
two new FastTriMeshSDF tests added (52 tests, all passing).

std::clamp: removed the custom scalar clamp() helper from
EBGeometry_AnalyticDistanceFunctions.hpp; its Vec3T overload now delegates to
std::clamp per component.  All scalar call sites updated to std::clamp: Capsule
and Cone SDFs in AnalyticDistanceFunctions, FiniteRepetitionIF in CSGImplem,
std::min(std::max()) patterns in DCEL_EdgeImplem and DCEL_VertexImplem, and
RandomCity examples.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UJSdT1HUkhxMw4DFYPZE3y
- Rename defaultK -> DefaultBranchingRatio
- Rename LinearUpdater/LinearSorter -> PackedUpdater/PackedSorter
- Rename equalCounts -> EqualCounts
- Rename sah2WaySplit/sahKWaySplitImpl -> SAH2WaySplit/SAHKWaySplit
- Remove virtual from TreeBVH destructor (no subclasses exist)
- Remove spurious BV2 template parameter from PackedBVH identity constructor
- Rename P2 -> PTree in PackedBVH converter constructor; expand @details
- Apply m_ prefix to PackedBVH::Node members (bv, primOff, numPrims, childOff)
- Apply m_ prefix to ChildAABBSoA members (lo, hi)
- Add static_assert(BV==AABBT<T>) to SAH2WaySplit and SAHKWaySplit
- Add EBGEOMETRY_EXPECT guards to all inline helper lambdas and Node::setChildOffset
- Add {} braces to all bare if-branches

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UJSdT1HUkhxMw4DFYPZE3y
bvhSDF and linSDF both called readIntoPackedBVH after the parser rename.
Drop the redundant bvhSDF slot; rename linSDF->meshSDF and update labels
to FlatMeshSDF / MeshSDF (PackedBVH) / TriMeshSDF (PackedBVH).
Also enable the FlatMeshSDF timing loop (was commented out) and add
relative-speedup lines for both accelerated representations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UJSdT1HUkhxMw4DFYPZE3y
Mirrors the existing operator-() so that +Vec3T<T>::max() compiles.
The unary plus is an identity operation that returns *this.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UJSdT1HUkhxMw4DFYPZE3y
rmrsk and others added 30 commits July 7, 2026 22:40
hi was computed from every building's getLowCorner() instead of
getHighCorner(), so the sampling bounding box undercounted the city's true
extent (missing each building's own width/length/height) -- discovered while
reviewing this example's documentation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Examples/ now only contains pure EBGeometry examples (no third-party
dependencies), so the EBGeometry_ prefix on each one was redundant --
EBGeometry_Shapes -> Shapes, EBGeometry_MeshSDF -> MeshSDF, and so on for
CSGUnion, PackedSpheres, RandomCity, and OctreeBoundingVolume. Updated every
CMakeLists.txt (project/target name), GNUmakefile (TARGET), README.md, and
all cross-references (CI.yml's directory matrices, Scripts/clang-tidy-check.sh,
CLAUDE.md, root README.md, Examples.rst, Introduction.rst).

AMReX is a target platform EBGeometry integrates with, not part of EBGeometry
itself, so Examples/AMReX_<something> moves to ThirdParty/AMReX/<something>
(one directory level deeper, so each GNUmakefile's AMREX_HOME default and each
main.cpp's #include "../../EBGeometry.hpp" needed an extra ../). The old
Chombo3_<something> examples (removed in eb1393f when the project dropped
Chombo support) are restored from that commit's parent under
ThirdParty/Chombo/<something>, with the same prefix-stripping and path-depth
fix applied.

Added ThirdParty/README.md: unlike Examples/, nothing under ThirdParty/ is
built or run in CI -- doing so would mean installing and tracking a specific
AMReX/Chombo version, which are independent projects with their own release
cadence. These are illustrative integration starting points, not maintained
or guaranteed to compile against the current release of their target
platform.

Also fixes a stale clang-tidy scoping regex (in both CI.yml and
Scripts/clang-tidy-check.sh) that matched literal path 'Examples/EBGeometry_'
-- after the rename this would have silently stopped analysing any example
code at all instead of erroring, so it's now scoped to 'Examples/' generally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ld methods

Previously the only way to compile a standalone example with runtime
assertions enabled was to fully override CXXFLAGS by hand (GNU make) or pass
a raw -DCMAKE_CXX_FLAGS (CMake) -- there was no dedicated option, unlike
PRECISION/EBGEOMETRY_PRECISION. Each GNUmakefile now has an ASSERTIONS=ON/OFF
variable (default OFF, matching the top-level project), and each standalone
CMakeLists.txt exposes the same EBGEOMETRY_ENABLE_ASSERTIONS option the
top-level project already defines (inherited as-is when built via
add_subdirectory, newly available when building an example on its own).
Direct compilation documents passing -DEBGEOMETRY_ENABLE_ASSERTIONS by hand,
consistent with how precision already works there. Verified all three build
methods for all six examples with the flag on and off.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
EBGeometry_CSG.hpp had zero behavioral test coverage -- UnionIF/BVHUnionIF
were only exercised indirectly via Examples (never run under sanitizers, and
CSGUnion's example doesn't cross-check correctness), while SmoothUnionIF,
IntersectionIF, SmoothIntersectionIF, DifferenceIF, SmoothDifferenceIF,
BVHSmoothUnionIF, and FiniteRepetitionIF were only ever compiled (via
InstantiateAll.cpp), never run.

Covers every class, every free-function wrapper, and the SmoothMin/SmoothMax/
ExpMin blending primitives, using pairs of analytic spheres (disjoint and
overlapping) chosen so expected values are hand-computable: sharp union/
intersection/difference against hand-derived max/min values; smooth variants
checked for the two properties that must hold by construction (never
shallower/never above the sharp result, and convergence to it once the two
inputs differ by more than the smoothing length); BVH-accelerated variants
cross-checked against their non-BVH counterparts over a 12-sphere row; and
finite repetition checked for exact periodicity within its range and correct
clamping to the boundary tile beyond it.

Passes under debug-san (ASan+UBSan) -- the first time this header's code has
run under a sanitizer at all -- and clang-tidy with zero new findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Quickstart section only shows building one example directly with g++;
there was no single place showing how to build and run the full test suite
plus every example at once. Adds the CMake preset workflow (configure, build,
ctest --preset debug, ctest --preset examples), with a pointer to the full
preset table and Scripts/run-all-checks.sh in the Sphinx docs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
EBGeometry_Transform.hpp had zero behavioral test coverage -- Translate,
Scale, Mollify, and Rotate were exercised only by the OctreeBoundingVolume
example (which asserts nothing about the transformed value, only prints a
bounding box), and Complement, Offset, Annular, Blur, Elongate, and Reflect
were never exercised outside InstantiateAll.cpp's compile-only coverage.

Covers every transform, mostly by cross-checking against an independently
built analytic shape with the same expected geometry (translated/scaled/
offset/mirrored sphere; an elongated sphere against sphere-at-clamped-center);
Rotate is checked via 360-degree and inverse-composition identities plus a
hand-verified 90-degree axis swap on an asymmetric box; Blur and Mollify are
checked via alpha=1/a_maxValue=0 exact-identity cases and, for the general
smoothing case, a constant-valued implicit function fixture that verifies
their sample weights are a true partition of unity (a weighted average of a
constant must return that same constant) without needing to hand-derive the
individual sample weights.

Passes under debug-san (ASan+UBSan) and clang-tidy with zero new findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…arning

The mismatch checks between FlatMeshSDF, MeshSDF, and TriMeshSDF printed a
warning to stderr but always returned 0, so ctest reported this example as
passed even if the accelerated representations silently disagreed with the
brute-force one -- exactly the kind of precision/SIMD correctness regression
this example is best positioned to catch. Now returns 1 on any mismatch,
matching the pattern PackedSpheres and RandomCity already use.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…meOctree gap

Neither EBGeometry_Octree.hpp's Node class nor
ImplicitFunction::approximateBoundingVolumeOctree (built on top of it) had
any Tests/ coverage -- the latter was only exercised by the
OctreeBoundingVolume example, which prints a bounding box but never asserts
it's correct.

Octree::Node is tested directly with a minimal plain-unsigned-int-level tree:
buildBreadthFirst/buildDepthFirst producing the expected 8^depth leaf count
and agreeing with each other, meta-data propagating correctly per generation,
traverse() correctly pruning a subtree via its Visiter and reordering
visitation via a custom Sorter.

approximateBoundingVolumeOctree is tested on an analytic sphere: the returned
bounding volume always contains the true bounds and tightens as tree depth
increases, a larger safety factor never shrinks the sphere out of the result,
and both documented error fallbacks (an inverted initial box, and an initial
box that never touches the surface) correctly return the maximal bounding
volume.

Passes under debug-san (ASan+UBSan) -- meaningful here given Node's
recursive shared_ptr/enable_shared_from_this structure -- and clang-tidy with
zero new findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
readIntoPackedBVH (used by the MeshSDF example but never in Tests/) and every
readInto*(std::vector<std::string>) overload -- readIntoDCEL, readIntoMesh,
readIntoPackedBVH, readIntoTriangleBVH -- had no coverage. Adds a direct
cross-check for readIntoPackedBVH against MeshSDF built from the same parsed
mesh, and verifies each multi-file overload is a faithful per-file loop (one
result per input file, each identical to what the corresponding single-file
call produces) using the dodecahedron fixture in two different formats.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Polygon2D was only exercised indirectly through DCEL::FaceT/MeshT's
point-in-face containment logic. Adds direct coverage for all three
inside/outside algorithms (winding number, crossing number, subtended
angle): a convex square, a concave L-shaped polygon (specifically checking a
point inside the vertices' convex hull but in the notched-out region), a
triangle embedded in a non-axis-aligned plane (exercising the
largest-normal-component projection-axis selection), and a grid sweep
checking all three algorithms agree with each other away from polygon
boundaries (where disagreement is expected and not meaningful to test).

Passes under debug-san (ASan+UBSan) and clang-tidy with zero new findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TriangleSoAT was only exercised indirectly through TriMeshSDF's internal
packing. Adds direct coverage: signedDistance() against the minimum-|value|
individual Triangle::signedDistance() for a fully-packed group (count == W),
a padded group (count < W, verifying the repeated-last-triangle padding
doesn't change the result), and a single-triangle group; computeBoundingVolume
against an AABB built directly from the same triangles' vertices.

Verified passing both under the debug preset (SIMD=none, scalar fallback
path) and release-test (SIMD=avx, exercising the actual AVX packed-double
intrinsics path for W=4/double) -- and under debug-san (ASan+UBSan), which
matters given the class's over-alignment requirements. Zero new clang-tidy
findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SimpleTimer was completely untested. Adds basic coverage: default
construction leaves it in a valid near-zero state, start()/stop() measures
at least the requested sleep duration, calling start() again resets
accumulated time, and a longer sleep yields a proportionally longer
measurement. Uses generous one-sided bounds throughout to avoid flakiness on
loaded CI runners.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…angle for float+double

First batch of converting the existing double-only Catch2 suite to
TEMPLATE_TEST_CASE(..., float, double), so behavioral correctness -- not just
compilation via InstantiateAll.cpp -- is actually verified under float.

WithinRel's type-specific overloads (100*epsilon<T> default) handle most
relative comparisons automatically once the argument is T rather than a
hardcoded double. WithinAbs has no such overload (only accepts double), so
absolute-zero comparisons now scale their own margin by
numeric_limits<T>::epsilon() -- tight for cases that are exactly
representable in either precision (e.g. a normal computed from axis-aligned
vectors), looser where a genuine sqrt/division makes the result only
approximately zero even in double.

Passes under debug-san (ASan+UBSan) and clang-tidy with zero new findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Introduces EBGEOMETRY_TEST_PRECISIONS (Tests/TestFloatingPointUtils.hpp), a
macro that expands to just `double` by default and to `float, double` when
Tests/CMakeLists.txt's new EBGEOMETRY_TEST_BOTH_PRECISIONS option is ON. Every
TEMPLATE_TEST_CASE in the suite is written as
TEMPLATE_TEST_CASE(..., EBGEOMETRY_TEST_PRECISIONS) instead of a hardcoded
`float, double`, so local iteration stays fast and matches whatever the CMake
preset otherwise builds, while CI (Unit-Tests, Sanitizers, Release-Test) now
passes -DEBGEOMETRY_TEST_BOTH_PRECISIONS=ON to verify float behaviorally on
every push.

Retrofits the first batch of templatized files (TestVec, TestBoundingVolumes,
TestAnalyticSDF, TestTriangle, TestPolygon2D, TestTriangleSoA, TestOBJ) onto
this shared header, replacing their per-file margin() duplicate helpers with
TestFloatingPointUtils.hpp's tightMargin<T>()/looseMargin<T>()/withinAbsT<T>()
(WithinAbs, unlike WithinRel, has no type-specific default margin and only
accepts double, so comparisons need an explicit, type-scaled margin either
way).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Continues the EBGEOMETRY_TEST_PRECISIONS conversion. TestSTL additionally
templatizes its two Parser::readSTL(...) file-parsing cases, verified against
the on-disk tetrahedron.stl fixture in both precisions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ug it found

approximateBoundingVolumeOctree's intersection test used a bare `1.0` literal
against a T-typed safety factor, silently double-promoting for T=float. This
went undetected because it's a member function *template* (BV is independent
of the class's own T) -- InstantiateAll.cpp only instantiates the class
itself, never actually calls approximateBoundingVolumeOctree, so its body was
never compiled for float before. Only caught once TestOctree's
TEMPLATE_TEST_CASE actually invoked it with T=float under
EBGEOMETRY_TEST_BOTH_PRECISIONS=ON.

Fixed by writing the literal as T(1.0) in both call sites
(EBGeometry_ImplicitFunctionImplem.hpp). The plain Octree::Node<unsigned int>
tests are left as ordinary TEST_CASE -- they have no floating-point
dependency at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cross-representation signed-distance comparisons (across file formats, BVH
build strategies, and BVH/FlatMeshSDF/MeshSDF/TriMeshSDF wrappers) needed
type-scaled tolerances rather than the shared tightMargin/looseMargin
helpers: formatMargin() for near-identical parses of the same geometry
(1e-9 double / 1e-4 float) and traversalMargin() for comparisons that chain
many more dot/cross products across the dodecahedron fixture's 36 faces
(1e-6 double / 1e-2 float). Verified against the default, AVX, and
debug-san builds, in both single- and both-precision configurations.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion bugs

BVHUnionIF::value and BVHSmoothUnionIF::value's BVH pruning visiters compared
a T-typed BV distance against a bare 0.0 literal, silently double-promoting
for T=float. Same root cause as the Octree fix: these are member function
bodies only ever instantiated when actually called, and nothing previously
called them with T=float. Fixed by writing T(0.0) in both visiters
(EBGeometry_CSGImplem.hpp).

Introduces three domain-specific margin helpers (exactMargin/formulaMargin/
asymptoticMargin) for the wide range of tolerance needs in this file: two
code paths computing the same closed-form expression, a hand-computed
formula result, and a smooth blend's asymptotic convergence to its sharp
counterpart, respectively -- each scaled up for float relative to the
existing double tolerances rather than reusing the generic shared-header
margins, since this file's comparisons span many different error
accumulation profiles.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same exactMargin/formulaMargin pattern as TestCSG, tailored to this file's
comparisons: two code paths computing the same closed-form expression
(exactMargin) vs. hand-computed formulas / partition-of-unity sums involving
a few more floating-point operations (formulaMargin). No source bugs found
this time -- verified in the default, both-precision, and debug-san builds.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Largest and last file in the float+double templatization pass. VertexT/
FaceT/EdgeT/MeshT/EdgeIteratorT/MeshSDF aliases and the loadTetrahedron()/
buildTetrahedron() fixture builders all become template aliases/functions
parameterized on T; every hardcoded `double` in the 55 test bodies becomes T.
Three margin tiers (exactMargin/formulaMargin/traversalMargin), matching the
naming already established in TestBVH.cpp, cover unit-normal/symmetric
checks, search-algorithm-agreement/near-zero-surface checks, and the
FastTriMeshSDF-vs-MeshSDF brute-force comparison respectively. No source
bugs found this time -- verified in the default, both-precision, and
debug-san builds.

This completes the EBGEOMETRY_TEST_PRECISIONS conversion of every Test*.cpp
file with a floating-point dependency (TestSFC.cpp and TestSimpleTimer.cpp
were already confirmed precision-independent and left untouched).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Catch2's WithinAbs/WithinRel matchers always operate on `double` internally
(MatcherBase<double>), regardless of which type-specific overload
constructed them. REQUIRE_THAT(<float value>, WithinRel(...)/WithinAbs(...))
therefore triggers an implicit float->double promotion *inside Catch2's own
header* (catch_matchers_impl.hpp's MatchExpr::MatchExpr), which
-Wdouble-promotion flags under Clang (confirmed with clang++-18 locally,
matching the clang++-14 CI failure) once EBGEOMETRY_TEST_BOTH_PRECISIONS
actually instantiates a TEMPLATE_TEST_CASE with T=float. GCC does not warn
on the same code, which is why this was missed locally.

This isn't a bug in EBGeometry's own code, so mark Catch2/Catch2WithMain's
include directories as INTERFACE_SYSTEM_INCLUDE_DIRECTORIES instead of
casting every REQUIRE_THAT argument to double at every call site. Verified
with clang++-18 (previously reproduced the CI failure exactly, now builds
and passes all 422 tests) and confirmed no regression in the default GCC
build or clang-tidy.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tolerance

The mismatch check compared meshSum/triSum against dcelSum with a fixed,
unscaled std::numeric_limits<T>::epsilon() -- effectively requiring
bit-for-bit identical sums. Summing 1000 signed distances via a brute-force
scan (FlatMeshSDF) and via BVH traversal (MeshSDF/TriMeshSDF) visits faces
in a different order, and floating-point addition isn't associative, so a
few-ULP discrepancy is expected and not a correctness bug. Confirmed by
reproducing the reported failure locally (a ~2^-36-scale diff) and finding
it recurs intermittently across runs with the same tolerance.

Switches to a relative check (no more than 1e-7 relative difference between
the sums, floored at magnitude 1) instead of a per-point comparison, which
would degrade the timing loops this example also serves as a benchmark for.
Verified with 20+ consecutive runs (double and float) against the armadillo
mesh with no false positives.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…shSDF

Both compared accumulated sums across representations (naive scan vs. BVH
union, and for PackedSpheres also a finite-repetition tiling) with exact
equality (sumSlow != sumFast) -- even more fragile than MeshSDF's original
epsilon()-based check, since it tolerates zero floating-point discrepancy
at all. Summing the same values in a different order is not bit-for-bit
reproducible, so this was liable to intermittent, environment-dependent
false failures for the same reason as the MeshSDF issue.

Applies the identical fix: a relative tolerance (1e-7, floored at magnitude
1) instead of exact/absolute comparison. Verified with multiple runs in
both double and float, via direct g++ invocation and the CMake build path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ble)

CI (Examples-FloatPrecision) failed on MeshSDF: TriMeshSDF vs FlatMeshSDF
diff = 0.015625 against a sum of ~1.3e5, a relative gap of ~1.2e-7 -- right
at float's own epsilon (~1.19e-7), just above the flat 1e-7 relative
tolerance introduced for the sum-comparison fix. float genuinely has less
headroom than double for the same "sum Nsamp terms in a different order"
argument, so a single tolerance value for both precisions was always going
to be borderline for float.

Fixes MeshSDF, PackedSpheres, and RandomCity (all three share this
sum-comparison pattern) to use 1e-4 relative tolerance for float and keep
1e-7 for double. Verified by reproducing the exact CI configuration (CMake
debug preset, EBGEOMETRY_PRECISION=float) locally: 10 consecutive MeshSDF
runs plus a full `ctest -L example` pass, all clean; confirmed no
regression in the default double-precision build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant