Skip to content

Update tutorials to match the current API - #247

Merged
zfergus merged 5 commits into
mainfrom
fix/update-tutorial
Aug 5, 2026
Merged

Update tutorials to match the current API#247
zfergus merged 5 commits into
mainfrom
fix/update-tutorial

Conversation

@zfergus

@zfergus zfergus commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

Went through all 11 pages in docs/source/tutorials and verified every code snippet against the current codebase. Verification was mechanical, not by eye: the C++ snippets were extracted into a compile harness (-fsyntax-only against the real headers, one translation unit per snippet so errors attribute precisely) and the Python snippets into a run harness against a built ipctk. 37 C++ snippets compile and 30 Python snippets run, where previously many did neither.

Removed or renamed API the tutorials still used

Tutorial said Actual API
ipc::point_triangle_ccd(...) free function method on NarrowPhaseCCD subclasses (TightInclusionCCD, AdditiveCCD, InexactCCD)
#include <ipc/ccd/ccd.hpp> header no longer exists
ipc::point_point_nonlinear_ccd + 3 siblings NonlinearCCD::point_point_ccd etc.
candidate.ccd(vertices, edges, faces, toi) candidate.ccd(dof(...), dof(...), toi) — takes stencil vertices
build(mesh, v, collisions, B, barrier_stiffness, mu) build(mesh, v, collisions, B, mu)
CollisionMesh(is_on_surface, positions, E, F) gained an orient_vertex mask
ProjectToPSD::CLAMP PSDProjectionMethod::CLAMP (and NONE was undocumented)
candidates.build(..., broad_phase) takes BroadPhase*, needs &broad_phase
ipctk.Collisions() ipctk.NormalCollisions()
collision_mesh.rest_positions() a property, not a method
initial_barrier_stiffness(..., max_barrier_stiffness) returns it instead of taking it

The TangentialCollisions::build one is worth calling out: in C++ the stale call still compiled, silently binding barrier_stiffness to mu_s and mu to mu_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::MatrixXd where MatrixXi was required, filib::Interval qualified as ipc::Interval, and several undefined or misspelled identifiers (mesh vs collision_mesh, collision vs collisions, 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_strategy instead inflates the minimum separation the query stops at:

min_effective_distance = (1 - conservative_rescaling) * (initial_distance - min_distance);
min_effective_distance = std::min(min_effective_distance, 1e-4);   // <-- usually binds

and only does toi *= conservative_rescaling when that first query returns toi < SMALL_TOI.

The practical consequence is worse than a wording nit: because the 1e-4 cap usually binds, conservative_rescaling of 0.8, 0.5, and 0.1 all return the byte-identical TOI 0.49994993209838867 for 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 nonzero min_distance.

Binding changes

Some Python tabs were unfixable as documentation because the API was not exposed:

  • SmoothCollisions.compute_adaptive_dhat — without this, adaptive dhat was unreachable from Python, even though build() takes use_adaptive_dhat=True and requires this be called first.
  • SmoothContactParameters.adaptive_dhat_ratio property.
  • BarrierPotential.stiffness / .use_physical_barrier properties, mirroring the C++ setters.
  • Renamed SmoothPotentialSmoothContactPotential to 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.0 scales the potential by exactly 3x, and for both use_physical_barrier values the potential, gradient, and Hessian are bit-identical to the constructor form.

Input validation instead of vanishing asserts

BarrierPotential asserts dhat > 0, stiffness > 0, and a non-null barrier, but assert() is compiled out under NDEBUG — so in a release build Python could set dhat = 0 and get undefined behavior instead of an error. The bindings now validate and raise ValueError, following the existing py::value_error convention in common.hpp. assert_positive is 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 (including dhat = 1e-300) still pass.

Test plan

  • 37/37 C++ tutorial snippets compile against real headers
  • 30/30 Python tutorial snippets run against a built ipctk
  • 24 existing python/tests pass (test_collision_mesh.py and test_ipc.py fail to collect on current pytest due to yield-style tests — pre-existing, untouched here)
  • nonlinear_ccd.rst literalinclude markers all still resolve; the test they pull from passes
  • clang-format clean; pre-commit hooks pass

🤖 Generated with Claude Code

zfergus and others added 2 commits August 4, 2026 16:28
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>
Copilot AI lite review requested due to automatic review settings August 4, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, BarrierPotential setters 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.

Comment thread python/src/common.hpp
Comment thread python/src/potentials/barrier_potential.cpp
@zfergus zfergus added this to the v1.6.1 milestone Aug 4, 2026
zfergus and others added 3 commits August 4, 2026 17:19
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

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.58%. Comparing base (57344ee) to head (f286192).

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     
Flag Coverage Δ
unittests 96.58% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zfergus
zfergus merged commit 3c317b5 into main Aug 5, 2026
21 checks passed
@zfergus
zfergus deleted the fix/update-tutorial branch August 5, 2026 06:10
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.

2 participants