Update tutorials to match the current API - #247
Merged
Conversation
Several parts of the documented C++ API had no Python equivalent, which made the GCP and convergent-formulation tutorials impossible to follow from Python: - Add SmoothCollisions.compute_adaptive_dhat. Without it, adaptive dhat was unreachable from Python even though build() accepts use_adaptive_dhat=True and requires this to be called first. - Add SmoothContactParameters.adaptive_dhat_ratio property. - Add BarrierPotential.stiffness and .use_physical_barrier properties, mirroring set_stiffness()/set_use_physical_barrier() in C++. Rename the Python SmoothContactPotential class from "SmoothPotential" to match the C++ name. It had no in-tree users and the package is still a 2.0 alpha, so this is a straight rename with no alias. Validate preconditions in the bindings rather than relying on the C++ asserts. BarrierPotential asserts dhat > 0, stiffness > 0, and a non-null barrier, but assert() is compiled out under NDEBUG, so a release build would silently accept a bad value and produce undefined behavior. The bindings now raise ValueError, following the existing py::value_error convention in common.hpp. The new assert_positive helper is written as !(value > 0) so NaN is rejected as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verified every snippet in docs/source/tutorials by extracting the C++ into a compile harness (-fsyntax-only against the real headers) and running the Python against a built ipctk. Both now pass end to end. Removed/renamed API the tutorials still used: - ipc::point_triangle_ccd and the other free narrow-phase functions are now methods on NarrowPhaseCCD subclasses; <ipc/ccd/ccd.hpp> no longer exists. - The four *_nonlinear_ccd free functions are now NonlinearCCD methods. - CollisionStencil::ccd takes stencil vertices, not (vertices, edges, faces); use dof() to gather them. - TangentialCollisions::build no longer takes barrier_stiffness. In C++ the stale call still compiled, silently binding barrier_stiffness to mu_s and mu to mu_k. Stiffness now comes from the normal potential. - CollisionMesh gained an orient_vertex mask, so the 4-argument construct_is_on_surface form no longer compiles. - ProjectToPSD is now PSDProjectionMethod (and NONE was undocumented). - Candidates::build takes a BroadPhase*, so the C++ call needs &broad_phase. - ipctk.Collisions does not exist; rest_positions is a property, not a method; initial_barrier_stiffness returns max_barrier_stiffness instead of taking it. Also fixed code that never worked: two Python snippets were SyntaxErrors (multi-line assignment without parentheses), a missing semicolon and a stray one, MatrixXd where MatrixXi/MatrixXd was required, filib::Interval qualified as ipc::Interval, and various undefined or misspelled identifiers (mesh vs collision_mesh, collision vs collisions, map_displacement). Corrected the note on conservative CCD. TightInclusionCCD does not scale the returned TOI in the normal path; it inflates the minimum separation the query stops at, capped at 1e-4, and only scales the TOI in the fallback taken when that query returns a TOI below SMALL_TOI. Because the cap usually binds, changing conservative_rescaling often has no effect on the result at all, which the previous wording actively obscured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Sphinx tutorials to match the current IPC Toolkit C++/Python APIs and extends the Python bindings where needed so that documented workflows (adaptive dhat, barrier parameter setters) are actually usable and fail safely in release builds.
Changes:
- Refactors multiple tutorial snippets to align with renamed/moved APIs (CCD, collision sets, collision mesh masks, PSD projection enum, candidates broad-phase pointer usage, etc.).
- Improves Python bindings for smooth contact / barrier potential (exposes
SmoothCollisions.compute_adaptive_dhat,SmoothContactParameters.adaptive_dhat_ratio,BarrierPotentialsetters and validation). - Clarifies Tight Inclusion CCD conservativeness behavior in docs (distance inflation vs TOI scaling fallback).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/src/potentials/barrier_potential.cpp | Adds Python-side validation and new/renamed bindings (BarrierPotential validation, SmoothContactParameters.adaptive_dhat_ratio, SmoothContactPotential rename). |
| python/src/common.hpp | Introduces reusable Python-binding validation helpers (assert_positive, assert_not_none). |
| python/src/collisions/normal/normal_collisions.cpp | Exposes SmoothCollisions.compute_adaptive_dhat to Python with docs and default args. |
| docs/source/tutorials/simulation.rst | Fixes tutorial code to use current mesh/collision/potential APIs and corrects types/identifiers. |
| docs/source/tutorials/ogc.rst | Updates tutorial snippets to pass collision_mesh where the API expects it (C++/Python). |
| docs/source/tutorials/nonlinear_ccd.rst | Updates nonlinear CCD documentation to the NonlinearCCD class API and fixes interval type qualification. |
| docs/source/tutorials/getting_started.rst | Updates examples for current CCD APIs, collisions class names, collision mesh properties, candidates build signatures, and improves CCD note accuracy. |
| docs/source/tutorials/gcp.rst | Updates docs to reference new Python property access for adaptive dhat ratio. |
| docs/source/tutorials/convergent.rst | Updates convergent formulation tutorial to use NormalCollisions and BarrierPotential.use_physical_barrier consistently (C++/Python). |
| docs/source/tutorials/adhesion.rst | Fixes minor tutorial snippet correctness (e.g., missing semicolon, updated tangential collisions build signature). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The input validation and newly exposed APIs added in 6a34858 had no automated coverage, so a regression would have been silent. Uses unittest.TestCase rather than plain functions so assertRaises is available under both nose2 (the CI runner) and pytest, without adding a pytest dependency. Covers: - BarrierPotential validation: ctor and setters reject <= 0 and NaN for dhat and stiffness, and None for barrier. Also asserts object state is unchanged after a rejected assignment, and that tiny-but-positive values still pass. - BarrierPotential.stiffness reaches the evaluation path, not just a stored field: tripling it triples the potential and gradient. - BarrierPotential.use_physical_barrier via the property is equivalent to the ctor kwarg for potential, gradient, and Hessian, plus a companion test that the flag changes the result at all so that equivalence is not vacuous. - SmoothCollisions.compute_adaptive_dhat as a differential pair: a baseline test pins that this mesh/dhat combination produces spurious nonzero forces at rest without adaptive dhat, and the adaptive test asserts they are exactly zero with it. The baseline is what keeps the second test meaningful. - SmoothContactParameters.adaptive_dhat_ratio round-trips and actually reaches compute_adaptive_dhat: larger ratios activate monotonically more collisions in a deformed configuration. - The SmoothContactPotential rename, guarded in both directions. Verified the tests bite by mutation testing: reverting the ctor validation, the setter validation, and the adaptive_dhat_ratio setter (to a no-op) turns them red, while the untouched barrier-ctor overload keeps passing, so the failures are specific rather than blanket. Potential/gradient/Hessian comparisons use a relative tolerance rather than exact equality, since the sums are parallel reductions whose operand order is not reproducible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Narrow-Phase section ended with "The alternatives are AdditiveCCD and
InexactCCD", which was wrong in three ways:
- InexactCCD is behind IPC_TOOLKIT_WITH_INEXACT_CCD, which defaults to OFF, so
it does not exist in a default build and is absent from the Python module.
Now marked opt-in, matching the wording already used in cpp-api/ccd.rst.
- It implied a difference in conservatism policy between the three that does not
exist. All three compute their margin as dmin + (1-r)(d0 - dmin);
TightInclusionCCD alone caps the second term at 1e-4, which is the entire
reason it reports a time of impact closer to the exact one for the same query.
- It said nothing about AdditiveCCD's actual trade-off.
For AdditiveCCD, lead with the strength (>100x faster, reliable in practice) and
keep the theoretical caveat subordinate: it does not account for rounding error
in its distance computations, but the default 10% margin is large enough to avoid
false negatives, at the cost of a less accurate time of impact and more false
positives. The failure mode is shrinking that margin, i.e. pushing
conservative_rescaling toward 1.0 — not ordinary use.
Also note that the margin is a fraction of the initial separation in excess of
dmin rather than of the raw distance. That distinction comes from the identity
documented on the gap computation in additive_ccd.cpp, (d - xi) =
(d^2 - xi^2) / (d + xi), and it is not cosmetic: for a large minimum separation
the two readings differ by an order of magnitude.
Normalize d_\text{min} to d_\min, the convention already used elsewhere in the
tutorials.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #247 +/- ##
=======================================
Coverage 96.58% 96.58%
=======================================
Files 163 163
Lines 16673 16668 -5
Branches 922 922
=======================================
- Hits 16103 16099 -4
+ Misses 570 569 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
zfergus
added a commit
that referenced
this pull request
Sep 3, 2026
…#249) * Template all distance functions for scalar type - Refactored all geometric distance, gradient, and hessian functions to be templated on scalar type (e.g., double, float). - Updated Python bindings to explicitly instantiate with double. - Removed point_point.cpp, moved implementation to header as inline template. - Updated all usages and tests to use templated versions. - Added Eigen expression overloads for point-point distance functions. - Updated normal and normalization utilities to be templated. - Set minimum C++ standard to C++20 in CMakeLists.txt. * Refactor smooth contact distance code to use unified distance functions - Remove redundant point/edge/triangle distance implementations from smooth_contact/distance - Replace calls to local *_sqr_distance with core distance functions - Update includes to use ipc/distance/point_line.hpp and related headers - Simplify mollifier and primitive_distance to use unified API - Minor cleanup in tangential_potential and point_plane distance math * Ensured all arithmetic in auto-generated and hand-written code uses the templated type for numeric literals * Templatize barrier functions and classes for float/double support - Convert barrier functions and barrier class hierarchy to use templates - Update all usages to specify template parameters where needed - Update Python bindings and tests to construct template barrier types - Extend benchmarks to compare float and double barrier performance - Improves support for mixed-precision and SIMD optimizations * Replace EigenExpression concept with typename - Set default CMAKE_CXX_STANDARD to 17 for top-level projects - Comment out EigenExpression concept in utils/eigen_ext.hpp - Update Eigen-expression wrapper templates to use typename parameters and add enable_if where appropriate * Fix template scalar typedef in edge-edge * Fix float precision and explicit-scalar calls in the templated distance API Scale PARALLEL_THRESHOLD in edge_edge_distance_type with the precision of T. u x v cancels for nearly parallel edges, leaving an absolute error of about eps*|u|*|v| per component, so sin^2(theta) cannot be resolved below ~eps^2. The threshold was left as a hard-coded 2.5e-16, which is ~1.13*eps for double but sits ~57x *below* float's noise floor, making the near-parallel branch unreachable in single precision. Over 20k exactly-parallel edge pairs the float distance type disagreed with the double one 44% of the time, and edge_edge_distance<float> differed from the double result by >0.1% relative in 14.8% of cases (worst case 24x). Both drop to 0% after the fix. The threshold is now derived from the double-tuned value by the ratio of epsilons, so the double threshold is bit-for-bit unchanged (asserted); only the newly added float instantiation changes. parallel_tolerance is typed T rather than double to match. Guard the *_distance_type EigenExpression wrappers with std::is_class_v. The other wrappers are rejected by SFINAE when the first template argument is given explicitly as a scalar, because their trailing return type mentions typename DerivedX::Scalar. These four return a non-dependent enum, so nothing rejected the candidate and substitution went on to form Eigen::MatrixBase<T>, a hard error inside Eigen rather than a substitution failure. As a result edge_edge_distance_type<double>(a, b, c, d) -- the natural way to pick the scalar in the new templated API -- failed to compile with 20 errors pointing into Eigen internals. The guard matches the one already used on the single-argument wrappers in geometry/normal.hpp. Drop the commented-out EigenExpression concept and its now-unused <concepts> include. The doc comment left above it described a dispatch contract the shipped code does not follow ("Layer 1 always calls Layer 2 with explicit <T>"), which is exactly the gap that produced the compile failure above. Fix the sizes in the MatrixMax alias doc comments: MatrixMax2f/MatrixMax2d were documented as 3x3 and MatrixMax9f/MatrixMax9d as 12x12. Add a v2.0.0 (alpha) section to the release notes covering the scalar templatization of the distance, barrier, and normal APIs, along with every commit since v1.6.0: the Tight Inclusion 1.1.0 update (#248), the tutorial and Python binding work (#247), the edge-triangle intersection coordinates and CollisionMesh::face_normals (#245), the mollified m == 0 Hessian PSD projection fix (#244), and the MeshFEM gallery entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Split the distance API into deducing front ends over fixed-size kernels The distance functions now have two layers. The concrete kernels moved into ipc::detail, templated on <typename T, int dim> (or just <typename T> for the 3D-only line_line, edge_edge, and point_triangle) and taking Eigen::ConstRef<Eigen::Vector<T, dim>>. The public names in ipc are thin front ends templated on the argument expression types: they deduce the scalar, resolve the dimension at compile time when the caller's type knows it, and otherwise take a single runtime branch on size(). Existing calls are unaffected. The win is the dimension, not the parameter passing. Erasing a Vector3d into a VectorMax3d cost 2.4x, so the front end branches on size() before it materializes anything and the fixed-size path never forms a dynamically sized temporary. Measured on the dominant in-tree call site, a row of a column-major MatrixXd: point_line_distance 8.2 -> 2.3 ns 3.5x point_edge_distance, AUTO 18.1 -> 5.2 ns 3.5x point_triangle_distance, AUTO 93.3 -> 12.8 ns 7.3x point_point_distance_hessian 5x normalization_and_jacobian 3.4x point_triangle_distance_type is the largest single piece of that: its three 2x2 LDLT solves are replaced by a closed form, since each edge lies in the triangle's plane and the Gram matrix is therefore diagonal. An error study over 10.8M configurations found no classification changes outside triangles collinear to within 1e-11 of their own edge length, a regime where the old code's own answer flips under a one-ulp input perturbation. It also fixes two real failures: above coordinate scale ~1e+51 the LDLT returned an infinite plane distance on every query, and in float near 1e-6 Eigen's tolerance discarded denormal Gram entries (2771 misclassifications per 400k, now 0). Two smaller levers, both measured: moving the cold throw bodies out of line behind [[noreturn]] helpers (worth up to 2x on its own, since constructing a std::invalid_argument inline consumes the caller's inlining budget), and branching once on the dimension in EdgeVertexCandidate (3.1x on the gradient). edge_edge_distance and point_triangle_distance keep their out-of-line switch dispatch; inlining a 9- or 7-case switch measured as a 10% regression with runtime distance types. line_line_distance_gradient and line_line_distance_hessian are single MatrixBase templates rather than two layers. They read three coefficients per argument and hand them to generated code, so Eigen::ConstRef's guaranteed single evaluation buys nothing and its materialization of an expression argument costs 1.16x. Also: - Guard the *_distance_type Eigen-expression overloads with std::is_class_v. Their return type is a non-dependent enum, so an explicit scalar argument was not rejected by SFINAE and instead formed Eigen::MatrixBase<double>, a hard error inside Eigen. - Scale PARALLEL_THRESHOLD in edge_edge_distance_type with the precision of the scalar type. The value tuned for double sits ~57x below the cancellation noise floor of u x v in single precision, making the near-parallel branch unreachable for float: across 20k exactly-parallel edge pairs the float distance type disagreed with the double one 44% of the time. The double threshold is bit-for-bit unchanged. - Taking the address of these functions is no longer possible, so the Python bindings wrap them in lambdas. - Rewrite tests/src/tests/benchmark_eigen.cpp around the shipped design: every row now compares the library against a same-TU reference implementation that acts as a noise-floor control, and the accumulated findings and measurement traps are consolidated into one header block. This drops a benchmark helper that asked for block<3,1> from a 1x3 row expression, reading out of bounds under NDEBUG. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Unify the non-floating-point distance-type guard and the autogen instantiations Two cleanups in the templated distance API. `edge_edge` and `point_triangle` guarded their AUTO resolution with `is_same_v<T, double> || is_same_v<T, float>` and then let AUTO fall through to the switch, where the default case threw the generic "invalid distance type". `point_edge` instead used `is_floating_point_v<T>` with an explicit `throw_auto_requires_explicit_dtype`. Adopt the latter everywhere, naming the function in the message, so a scalar type that cannot resolve a distance type now reports why rather than claiming its distance type is invalid: edge_edge_distance: an explicit distance type is required for non-floating-point scalars; ... This is a user-visible improvement for autodiff scalars, which reach these paths today. It also covers the gradient and Hessian kernels, whose AUTO resolution was previously unguarded, so they can now be instantiated for scalar types that have no ordering. The generated `autogen` instantiations in `line_line`, `point_line`, and `point_plane` spelled out every signature once per scalar type -- eight hand-written lines in `point_line`, whose four functions take 6, 9, 6 and 9 arguments and fill arrays of 6, 9, 36 and 81. Fold each file's list into an `IPC_INSTANTIATE_*_AUTOGEN(T)` macro, matching the convention already used for the distance kernels themselves. `nm` confirms the same sixteen `float` and `double` symbols are still emitted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Name every unsupported scalar family in the AUTO distance-type error The message claimed "the distance type cannot be determined from an autodiff scalar", which predates the other scalar types that reach this path. It is now also hit by SIMD batches and by filib::Interval, for which the old wording was simply wrong. Say what the actual obstacle is -- resolving AUTO means comparing single ordered values -- and name all three families. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Extend the two-layer template design across the remaining geometry kernels Convert every remaining per-collision kernel family to the shape the distance functions established: fixed-size inner kernels in ipc::detail, deducing expression-templated front ends in ipc, and float alongside double. Families that already had scalar-templated kernels but no front end (so a matrix-row argument forced a Ref materialization per call, and an explicit scalar produced an error wall inside Eigen): edge_edge_mollifier, point_plane, and the three signed distances (line_line, point_line -- a 2D family -- and point_plane). Families that were double-only with dimension-erased VectorMax/MatrixMax signatures: closest_point, tangent_basis, relative_velocity, and geometry/area. Their autogen kernels are templated with the same instantiation-macro pattern as line_line; the only edits to generated expressions are T() literal wrappers, verified byte-identical to the previous text modulo the scalar substitution. relative_velocity's runtime-dim entry points keep their exact public signatures (template <typename T = double> over dim-templated kernels), so call sites passing int dim compile unchanged. No caller anywhere needed editing; the deducing front ends accept every argument type used in-tree. relative_velocity ends up fully header-inline, so its TU is deleted. Small hand-written kernels are header-inline; wide autogen bodies stay in their TUs. That split is load-bearing, and the new "Converted families" benchmark (tests/src/tests/benchmark_eigen.cpp) measured it both ways: with the tangent kernels TU-defined, the dimension-erased path paid the Ref copies and MatrixMax wrap at an opaque call boundary and point_edge_tangent_basis REGRESSED 2x on matrix rows; header-inlining the value kernels turned that into the wins below. Measured old-vs-new by interleaved A/B of two binaries (baseline built from a worktree at the previous commit; controls 0.99-1.00x): point_edge_closest_point, rows 8.7 -> 1.8 ns 4.8x point_edge_closest_point, Vector3d 4.7 -> 1.7 ns 2.8x point_edge_closest_point_jacobian, rows 14.9 -> 6.2 ns 2.4x point_point_relative_velocity, rows 5.5 -> 1.4 ns 3.9x pp_relative_velocity_jacobian(dim) 6.8 -> 1.8 ns 3.9x edge_length, rows 4.1 -> 1.3 ns 3.1x point_edge_tangent_basis, rows 7.1 -> 4.2 ns 1.7x point_triangle_tangent_basis, rows 7.7 -> 5.0 ns 1.5x The already-fixed-size 3D functions (edge_edge/point_triangle closest point, triangle_area, edge_edge_cross_squarednorm, point_plane_distance) measured neutral, as expected: the win was always recovering the compile-time dimension, not the templates themselves. One residual: point_point_tangent basis on matrix rows reads 0.87-0.90x (its fixed-dim path improved 1.11x); suspected branch/layout effects in its axis-picking body, unresolved. Also: move the relative-velocity Γ layout derivations from stranded TU comments into the doxygen of their public front ends, exempt NOTE/TODO/ WARNING/FIXME comments from clang-format reflow, and swap a leftover fmt include for spdlog in the CCD benchmark test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bind the distance and tangent kernels directly in Python The public front ends deduce their scalar and dimension from the argument expressions, so their address cannot be taken. Rather than wrap each one in a lambda, bind the ipc::detail kernel with its scalar named explicitly -- &detail::f<double> -- which is a concrete function and needs no wrapper. Two cases keep something else: - point_plane_distance, _gradient and _hessian each have two arities (a plane given as origin+normal, or as three triangle vertices). Both remain bound under one Python name, disambiguated with py::overload_cast on the parameter list. - edge_length_gradient, point_point_relative_velocity and point_edge_relative_velocity keep their lambdas. Their kernels are templated on the dimension, so any address-of would pin one; the lambdas take VectorMax3d and preserve the runtime 2D/3D dispatch these functions have always offered from Python. Docstrings, argument names and Python-visible signatures are unchanged. Verified against a freshly built ipctk (the first Python build of this work): nose2 -s python/tests passes 95/95, and a targeted probe exercises all 40 converted entry points. The repo suite covers none of these functions, so the probe is what actually checks them: both point_plane arities return the right shapes (3-vector/3x3 versus 12-vector/12x12), the three retained lambdas still accept 2D and 3D (shape (4,) versus (6,) from edge_length_gradient), the runtime-dim jacobians work at dim 2 and 3, and values are correct -- signed distance -1 where the unsigned distance is +1, edge-edge cross squared norm 16 for perpendicular length-2 edges. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Bind the remaining 3D distance kernels directly in Python Applies the &detail::f<double> treatment to the bindings converted to lambdas earlier on this branch: line_line, edge_edge and point_triangle. Their kernels take Eigen::Vector3<T>, so naming the scalar is enough to get a concrete function and the wrapper is unnecessary. The dtype parameters keep their py::arg defaults, which bind fine against the kernel's required parameter. Three groups deliberately keep their lambdas: - line_line_distance_gradient and line_line_distance_hessian have no detail kernel to address. They are single-layer Eigen::MatrixBase templates, which is correct for them: they read three coefficients per argument and hand them to generated code, so there is no whole-vector operation whose size a second layer would need to recover. - point_point, point_line, point_edge and point_edge_distance_type take VectorMax3d and dispatch on the dimension at runtime. - The normalization_* family, likewise VectorMax3d. Pinning a dimension in any of those would silently drop 2D from the Python API. Verified against a rebuilt ipctk: nose2 95/95, the previous 40-point probe still green, and a new 21-point probe covering the newly converted functions and the retained lambdas. Both dtype spellings (defaulted and explicit) return the same value, and every VectorMax3d entry point still answers in 2D and 3D. Docstrings, argument names and signatures unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Document the geometry-kernel conversion in the release notes Covers the three preceding commits: the two-layer conversion of the tangent, closest-point, relative-velocity, area, mollifier, point-plane and signed distance families; the measured speedups and the one unresolved regression; the header-inline versus translation-unit split and why it is load-bearing; and the Python bindings moving from lambdas to &detail::f<double>. Also corrects two claims that the intervening work invalidated: the Highlights line named only the distance, barrier and normal APIs, and the API Changes section still advised wrapping in a lambda as the only way to take a function's address. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Rename NormalizedBarrier's private scalar alias to avoid an MSVC clash The private 'using T = typename BarrierT::value_type' shadowed the template parameter of any derived class that also names its parameter T. MSVC resolved 'NormalizedClampedLogBarrier<T>' inside PhysicalBarrier to the inherited private typedef and rejected it as inaccessible, while Clang and GCC resolved it to the derived class's own parameter. * Allow the _v variable-template suffix in the clang-tidy naming check is_eigen_expression_v, are_eigen_expressions_v, and dim_v follow the standard library's convention for variable templates, which the UPPER_CASE GlobalConstantCase rule rejects. * Match the CI clang-format version to the pinned pre-commit hook .pre-commit-config.yaml pins mirrors-clang-format v21.1.2, but the format check ran clang-format 20. The two disagree on how to pack the MatrixMax3 initializers in normal.hpp, so a file formatted by the hook a contributor actually runs failed CI. The whole tracked tree is already clean under 21. * Key the CI build cache on the runner CPU FindSIMD compiles with -march=native, so every cached object file carries the building runner's ISA. The cache key was runner.os + config with no CPU component, so objects built on one runner model were restored onto another and the test step died with SIGILL across unrelated suites (friction, candidates, CFL, plane-vertex collisions). The run that populated the cache compiled from scratch in 4m58s and passed; the next run restored 313 MB, built in 1m22s, and failed in 29s. * Clean up release_notes.rst. * Move low-level normal/distance templates into ipc::detail behind SFINAE-friendly front ends Push the fixed-size, per-scalar implementations (point-line/triangle/line-line normals, point/edge/triangle/plane distances, tangent bases, relative velocities) into ipc::detail, and give each a thin ipc:: front end that takes Eigen::MatrixBase<Derived> so overload resolution SFINAEs away cleanly instead of hard-erroring on non-Eigen arguments. Drops the IPC_ASSERT_EIGEN_ARGS macro and its is_eigen_expression_v/are_eigen_expressions_v traits now that the MatrixBase parameter does that job directly. Copies the detail-side Doxygen comments down to the public wrappers so the documented API carries its own docs instead of pointing into an internal namespace. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix documentation - Explicitly specify the global templated version of distance functions --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Went through all 11 pages in
docs/source/tutorialsand verified every code snippet against the current codebase. Verification was mechanical, not by eye: the C++ snippets were extracted into a compile harness (-fsyntax-onlyagainst the real headers, one translation unit per snippet so errors attribute precisely) and the Python snippets into a run harness against a builtipctk. 37 C++ snippets compile and 30 Python snippets run, where previously many did neither.Removed or renamed API the tutorials still used
ipc::point_triangle_ccd(...)free functionNarrowPhaseCCDsubclasses (TightInclusionCCD,AdditiveCCD,InexactCCD)#include <ipc/ccd/ccd.hpp>ipc::point_point_nonlinear_ccd+ 3 siblingsNonlinearCCD::point_point_ccdetc.candidate.ccd(vertices, edges, faces, toi)candidate.ccd(dof(...), dof(...), toi)— takes stencil verticesbuild(mesh, v, collisions, B, barrier_stiffness, mu)build(mesh, v, collisions, B, mu)CollisionMesh(is_on_surface, positions, E, F)orient_vertexmaskProjectToPSD::CLAMPPSDProjectionMethod::CLAMP(andNONEwas undocumented)candidates.build(..., broad_phase)BroadPhase*, needs&broad_phaseipctk.Collisions()ipctk.NormalCollisions()collision_mesh.rest_positions()initial_barrier_stiffness(..., max_barrier_stiffness)The
TangentialCollisions::buildone is worth calling out: in C++ the stale call still compiled, silently bindingbarrier_stiffnesstomu_sandmutomu_k. Anyone copying that snippet got a wrong friction coefficient with no diagnostic.Code that never worked
Two Python snippets were outright
SyntaxError(multi-line assignment without parentheses). Also a missing;and a stray one,Eigen::MatrixXdwhereMatrixXiwas required,filib::Intervalqualified asipc::Interval, and several undefined or misspelled identifiers (meshvscollision_mesh,collisionvscollisions,map_displacement).Corrected the conservative-CCD note
The note claimed the returned TOI "is scaled by
DEFAULT_CONSERVATIVE_RESCALING". That describes a fallback branch, not the normal path.ccd_strategyinstead inflates the minimum separation the query stops at:and only does
toi *= conservative_rescalingwhen that first query returnstoi < SMALL_TOI.The practical consequence is worse than a wording nit: because the
1e-4cap usually binds,conservative_rescalingof0.8,0.5, and0.1all return the byte-identical TOI0.49994993209838867for the tutorial's own query. Someone tuning that parameter to tighten the result would see nothing change and reasonably conclude the knob was broken. The note now gives the formula, flags the cap, and scopes the TOI-scaling claim to the fallback. The formula was validated against the implementation across 7 configurations, matching to 6 decimal places including nonzeromin_distance.Binding changes
Some Python tabs were unfixable as documentation because the API was not exposed:
SmoothCollisions.compute_adaptive_dhat— without this, adaptivedhatwas unreachable from Python, even thoughbuild()takesuse_adaptive_dhat=Trueand requires this be called first.SmoothContactParameters.adaptive_dhat_ratioproperty.BarrierPotential.stiffness/.use_physical_barrierproperties, mirroring the C++ setters.SmoothPotential→SmoothContactPotentialto match C++. No in-tree users and the package is a 2.0 alpha, so it is a straight rename with no alias.Verified the new setters reach the evaluation path rather than just storing a field: setting
stiffness = 3.0scales the potential by exactly 3x, and for bothuse_physical_barriervalues the potential, gradient, and Hessian are bit-identical to the constructor form.Input validation instead of vanishing asserts
BarrierPotentialassertsdhat > 0,stiffness > 0, and a non-null barrier, butassert()is compiled out underNDEBUG— so in a release build Python could setdhat = 0and get undefined behavior instead of an error. The bindings now validate and raiseValueError, following the existingpy::value_errorconvention incommon.hpp.assert_positiveis written as!(value > 0)so NaN is rejected too. Confirmed against a release (NDEBUG) build that all 15 invalid inputs raise, object state is unchanged after a rejected set, and valid values (includingdhat = 1e-300) still pass.Test plan
ipctkpython/testspass (test_collision_mesh.pyandtest_ipc.pyfail to collect on current pytest due toyield-style tests — pre-existing, untouched here)nonlinear_ccd.rstliteralincludemarkers all still resolve; the test they pull from passesclang-formatclean; pre-commit hooks pass🤖 Generated with Claude Code