NanoVDB Math: refactor onto MatBase/VecBase, fix correctness bugs, add tests

[swahtz]

Consolidates nanovdb/math/Math.h by lifting Mat / Vec arithmetic onto two shared base classes (MatBase<T, R, C> and a new VecBase<T, N>). The five Mat and three Vec derived classes become thin delegating wrappers. The pass also fixes several long-standing correctness bugs and fills inconsistent / partial APIs.

Files changed: Math.h (the refactor), NanoVDB.h (Map API now constexpr — depends on the matMult rework), tools/cuda/PointsToGrid.cuh (three call-site casts from the explicit ctor tightening), and three test files (additive: 11 new TEST_F blocks plus mechanical cast updates), and one python-binding kernel.

Bug fixes

• math::Min / Max for int32_t / uint32_t no longer route through fminf / fmaxf — old form lost precision above 2^24 and could be UB near INT_MAX. Heavily used by HDDA.h.
• Removed dimensionally-invalid operator*(Mat3, Mat2x3) -> Mat2x3. No callers in the tree.
• Mat2::inverse() now returns an explicit zero on singular input (was uninitialised) and uses T(1)/det for T=double.
• Vec*::operator/(T) and Mat*::operator/(T) now per-element-divide. The old (T(1)/s) * (*this) did integer division for integer T, so Vec3i(6,8,10) / 2 returned (0,0,0).
• Vec2's operator+/-/+=/-= against Coord now take Coord2. Old signature silently dropped z.
• math::Round(Vec3<float>) and Round(Vec3<double>) both use floor(x + 0.5) now. The float overload previously used rintf (round-half-to-even), so the two disagreed at half-integers.
• Corrected floor() / ceil() doc swap on Vec2 / Vec3. Implementations were always right.

Refactor

• MatBase<T, R, C>: shared element-wise / scalar / equality / transpose / matmat / matvec implementations, all __hostdev__ constexpr. rows() / cols() / size() promoted public.
• VecBase<T, N> (new): shared element-wise / scalar / equality / reductions / mutating min/max / integer rounding. floorAs<Result> / ceilAs<Result> / roundAs<Result> unify Round semantics across float / double / long double.
• All five Mat and three Vec classes become thin wrappers: ctors + per-type extras (Mat2::inverse, Vec3::cross/outer) + one-line operators delegating to the base.
• Mat3 / Mat3x2 / Mat4 scalar ctors switched from template<typename Source> to T, matching Mat2 / Mat2x3. Mixed-literal construction (Mat3<double>(1, 2.0, 3, ...)) now works uniformly. Array ctors keep template<typename Source> for cross-type interop.

API completeness

• Every Mat type has the same operator surface: unary -, binary + / -, += / -=, *(T) / /(T), *=(T) / /=(T), == / !=. Previously scattered.
• New mat*mat: Mat3 * Mat3x2 standalone, Mat4 * Mat4 member.
• New mat*vec members: Mat2 * Vec2, Mat2x3 * Vec3 -> Vec2, Mat3x2 * Vec2 -> Vec3. Mat4 * Vec4 promoted to member.
• New scalar * MatN standalones for Mat2x3 / Mat3x2 / Mat3 / Mat4.
• Vec4 gains parity with Vec3: min, max, asPointer, floor, ceil, round (returns Vec4<int32_t> — no Coord4).

constexpr sweep

Every function in Math.h and the Map API in NanoVDB.h that C++17 allows is now constexpr. Carve-outs: length / normalize (std::sqrt, C++26), the float / double overloads of Min / Max / Floor / Ceil / Round / Abs (C++23), and CUDA-device-only *Atomic helpers.

Rewriting matMult / matMultT from fma(a, b, c) to a * b + c was required (fma isn't constexpr until C++23, P1383R2). GPU codegen unchanged — nvcc -fmad=true contracts back to hardware FMA. Host codegen unchanged under default -ffp-contract=on. Only compile-time constexpr evaluation observes the double-rounded result. Worst-case ≤1 ulp drift, well below NanoVDB's geometric precision.

Other quality passes

• Bounds checks on every operator[] in Math.h via NANOVDB_ASSERT (twelve sites). No-op under NDEBUG; const-index accesses fold at compile time even in debug.
• Doxygen sweep: every previously-undocumented public method gains an @brief; several // @brief typos (missing leading slash — silently invisible to Doxygen) fixed.
• Hidden friends: scalar*Vec / scalar/Vec / scalar*Mat converted from namespace-scope free templates to in-class friend definitions — ADL-found only, no longer pollute the enclosing namespace.

Tests

Eleven additive TEST_F blocks in TestNanoVDB.cc:

Test	Covers
IntegerMinMax	2^24+1 precision; INT_MAX / UINT_MAX overflow safety.
Round	Half-integer agreement between float and double (especially -1.5).
Vec2 / Vec3Ops / Vec4Ops	Full operator coverage; new Vec4 methods; integer-T division.
Mat2 / Mat3 / Mat4 / Mat2x3_Mat3x2	Constructors, every operator, transpose, matmat, matvec; Mat2::inverse non-singular roundtrip + singular-returns-zero; mixed-literal construction.
MatMul	Every dimensionally-valid matmat plus every member matvec across the five matrix types.
MatVecIntrospection	static_asserts for rows/cols/size/SIZE/ValueType; data()/asPointer() ordering.

Risk and compatibility

• Behaviour preserved for every previously-correct code path.
• Behaviour changes are exactly the bug fixes above — each moves an incorrect / undefined output to a defined, correct one.
• matMult rounding: ≤1 ulp drift only in pure abstract-machine evaluation. GPU and host codegen identical to before.
• API additions are pure. The only removed overload is the broken Mat3 * Mat2x3 (no callers).
• One signature tightening: Vec2's Coord overloads now require Coord2. Code passing a 3D Coord to Vec2 will fail to compile — but it was silently dropping z.
• Cross-template ctor tightened to explicit: Vec*(const Vec*T<T2>&) now matches the same-class ctor's convention. Audit found 18 affected sites across the nanovdb + openvdb trees, all updated here (3 in PointsToGrid.cuh, 3 in TestNanoVDB.cu, 12 in TestOpenVDB.cc, 1 in PySampleFromVoxels.cu). All mechanical and value-preserving. Downstream consumers that relied on the implicit conversion will need the same one-line cast — typically nanovdb::Vec3f(yourVec) or an explicit template argument to a returning helper.

[Copilot]

Pull request overview

This PR refactors NanoVDB’s math/Math.h by consolidating shared vector/matrix arithmetic into new VecBase<T, N> and enhanced MatBase<T, R, C> base classes, while also fixing several long-standing correctness issues and adding unit tests to lock in the corrected behavior.

Changes:

• Refactor vector and matrix implementations onto VecBase/MatBase helpers to remove duplicated per-element loops and normalize APIs across types.
• Fix correctness issues (e.g., integer Min/Max, consistent Round semantics, singular Mat2::inverse, scalar division for integer Vec*/Mat*, invalid mat-mul overload removal).
• Add 11 new unit tests covering the refactor, new APIs, and each bug fix with numeric checks and compile-time introspection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
nanovdb/nanovdb/math/Math.h	Introduces VecBase, expands MatBase, rewrites Vec*/Mat* as thin wrappers, fixes math correctness bugs, and rationalizes overload sets.
nanovdb/nanovdb/unittest/TestNanoVDB.cc	Adds new TEST_F blocks that exercise the refactored math types and pin down the corrected semantics.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[harrism]

This is a really great cleanup and improvement. I have comments in 4 areas.

1. constexpr -- might as well constexpr all the things
2. Alignas -- on the classes where it makes sense, let's guarantee alignment so the compiler can optimize / coalesce loads (this allows each row of a 4x4 matrix to be loaded with a single 128-bit global load, for example. Without it, the compiler will generate 4 32-bit loads).
3. Other modernization / best practices topics: [[nodiscard]], noexcept, consistent explicit ctors, hidden friends.
4. Documentation -- didn't comment on this much inline, but make sure all methods have doxygen docs. Looks a bit inconsistent?

[harrism]

I also asked an agent to review with modern C++ best practices in mind and then had it prune its list to not include what I already mentioned in my review.

This first one would be good to implement.

 Body-assignment in derived constructors. The new derived ctors do this->mVec[0] = x;
  this->mVec[1] = y; instead of the pre-PR : mVec{x, y} {}. Invisible for fundamental T, a
  regression for any future Vec<UserType>. Fix is a protected variadic constructor on VecBase:

  template<typename... Args>
  __hostdev__ explicit VecBase(Args... args) : mVec{T(args)...} {
      static_assert(sizeof...(Args) == N, "wrong number of args");
  }

  Derived ctors delegate: Vec2(T x, T y) : Base(x, y) {}.

The rest are all speculative / nice to have or depend on C++20 or later.

Defaulted operator== (C++20 only). bool operator==(const Vec3&) const noexcept = default;
  replaces the manual equals() chain; operator!= auto-synthesizes. Only worth raising if NanoVDB's
   C++ baseline allows.

  Tuple-like interface (tuple_size / tuple_element / get) for auto [x, y, z] = v; and std::apply.
  C++17.

  begin()/end() so Vec/Mat work with range-based for and <algorithm>. Six lines per base.

  [[deprecated]] on the lowercase size alias if the goal is to migrate callers to SIZE. Nudges
  without breaking.

  static_cast<T>(x) over functional-style T(x). Identical for arithmetic types but greppable and
  doesn't silently become a constructor call for non-arithmetic T. Minor.

  CTAD deduction guides so Vec3 v(1.0f, 2.0f, 3.0f); deduces Vec3<float>. Three lines per Vec /
  Mat class. C++17.

  C++20 requires clauses on cross-Vec templated functions (dot<V>, mergeMin<V>, the templated
  converting constructor). Replaces static_assert(V::SIZE == N) in the body with constraints at
  the declaration. Better diagnostics, SFINAE-friendly.

  std::array<T, N> instead of T[N] in the base. Free .data(), iterators, structured bindings,
  .size(), .fill(), ==. Layout-compatible with T[N] for fundamental T. Possibly tangles with CUDA
  portability (cuda::std::array exists). Out of scope for this PR; worth knowing.

[swahtz]

Implemented in a1072df. Added a protected variadic ctor to both VecBase and MatBase and converted every derived Vec/Mat ctor (Vec2/3/4, Mat2/2x3/3x2/3/4) to delegate via initializer list — no more body-assignment, no more default-construct-then-overwrite of the base subobject. The static_assert(sizeof...(Args) == N) (and == ROWS*COLS for MatBase) catches arity mismatches at the call site.

Tracking the other suggestions for follow-up but not in this PR — happy to file separate issues for the C++17-feasible ones (tuple-like interface, begin/end, CTAD guides) if useful.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

[harrism]

Fantastic. Thanks for addressing all my suggestions.