Extremum Sum Potential - #259
Draft
fsichetti wants to merge 254 commits into
Draft
Conversation
# Conflicts: # src/ipc/high_order_contact/collisions/high_order_collision_template.cpp # src/ipc/high_order_contact/high_order_contact_potential.cpp # tests/src/tests/potential/test_high_order_potential.cpp
Merges 64 upstream commits into the fork. 14 files conflicted; notable resolutions: - collision_mesh.hpp: adopt upstream's CollisionFilter for can_collide (default_can_collide was removed upstream); keep our is_watertight(). - distance_type.cpp: keep our predicate/standard split, port upstream's edge-edge parallel-threshold fix (1e-20 -> 2.5e-16, relative a*c scaling) into the standard path. - normal_collisions_builder.cpp: upstream refactored the batch builders to single-candidate functions; our skip_obstacles/use_ogc logic ported in (continue -> return). weight stays non-const for obstacle half-weighting. - barrier_potential: merged constructor takes both upstream's stiffness and our use_squared_distance. - candidates.cpp: clear() clears our ef/ff plus upstream's pv. Follow-on fixes required by upstream changes: - Replace maybe_parallel_for with direct TBB APIs across high_order_contact/ (upstream removed the wrapper in ipc-sim#214; the IPC_TOOLKIT_WITH_TBB guard it relied on was always defined, so the serial branch was dead code). - Port edges_to_faces call sites to the new vector<vector<index_t>> representation. - Add the pv_collisions structured binding in the high-order TangentialCollisions::build overload. - Drop an extra semicolon now caught by -Werror=extra-semi. - test_barrier_potential.cpp: take upstream's removal of the dead all_vertices_on_surface branch (it was hardcoded true). Tests: 317 passed, 1 skipped (pre-existing, missing cube.obj), 7159460 assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rallel test Three related changes to distance_type and its tests. 1. IPC_TOOLKIT_WITH_GEOGRAM=OFF now builds. Previously the whole of distance_type.cpp sat inside #ifdef IPC_TOOLKIT_WITH_GEOGRAM with #else -> #error "NOT IMPLEMENTED!", and the geogram headers were included unguarded above that anyway, so the option could only ever be ON. Now only the geogram-dependent pieces are guarded (exact types, the filtered sign predicates, the three _predicate distance-type implementations, and the exact branch of is_parallel_edge_edge); the _standard implementations are always compiled and the dispatchers fall back to them when geogram is absent. IPC_TOOLKIT_WITH_GEOGRAM moves from a PRIVATE target_compile_definitions to config.hpp, matching every other option, so tests can see it too. The four exact-reference test cases are guarded accordingly. Verified: library and tests build with -DIPC_TOOLKIT_WITH_GEOGRAM=OFF and the distance-type tests pass (7 cases, the 4 exact-comparison ones being geogram-only). 2. Single parallel threshold. PARALLEL_THRESHOLD is now 2.5e-16 scaled by a*c, matching upstream, and is used by both is_almost_parallel_edge_edge and the standard edge-edge classifier. The local STANDARD_PARALLEL_THRESHOLD is gone. NOTE: this tightens is_almost_parallel_edge_edge on sub-unit edges (at a*c = 1e-3, ~400x). Its only callers are the two high-order builder sites, which no test exercises, so the suite passing does not validate it. Revert this hunk alone if the high-order path regresses. 3. The edge-edge parallel test now uses a genuinely exact reference. edge_edge_distance_type_exact takes a parallel_threshold parameter defaulting to PARALLEL_THRESHOLD; the parallel test passes 0. With a non-zero threshold the reference is a hybrid: exact arithmetic but an approximate parallelism test, which routed merely near-parallel edges to edge_edge_parallel_distance_type_exact. That classifier is only valid for exactly parallel edges, so the reference returned a strictly larger distance than the true minimum -- verified in exact rational arithmetic, where the shipped predicate returns the true minimum and only one of the eight predicate conditions is ever satisfied. The failures were therefore in the reference, not the classifier. The test is no longer tagged [.]: 202998 failing assertions -> 0, all 1104000 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The subtree builds on libc++ and libstdc++ but has never been built with MSVC, which fails it in three independent ways. All three are portability bugs rather than anything to do with the maths. 1. `double near = 0, far = 0;` in quadrature_potential.cpp. `near` and `far` are MACROS from Windows' windef.h -- legacy 16-bit memory-model keywords MSVC still defines -- so the declaration expands to `double = 0, = 0;` and the compiler reports `C2513: 'double': no variable declared before '='` followed by a cascade of syntax errors. Renamed to near_sum/far_sum, which are local variables in three functions and visible to nobody. (NearFarBarrier::near/::far are left alone: they are member functions, so the macro cannot reach them through a `->` and no translation unit that calls them currently fails.) 2. M_PI in high_order_quadrature.hpp. Not standard C++: MSVC defines it only when _USE_MATH_DEFINES is set before <cmath>, which a consumer of a public header cannot be relied on to do. Replaced with a constexpr in namespace ipc. 3. std::array and std::uint*_t used across the subtree without including <array>/<cstdint>. libc++ and libstdc++ supply both transitively through other standard headers; MSVC's do not, so e.g. high_order_contact_parameters.hpp fails with `C2079: 'ipc::FaceQuadPoint::lambda' uses undefined class 'std::array<double,3>'`. Added the includes to the nine headers and sources that need them. Found by building wildmeshing-toolkit against this subtree on the Windows CI runners, where each fix exposed the next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This flag switched the high-order-contact collision building and potential evaluation to an alternate per-vertex "feasible region" code path replicating the OGC paper's algorithm directly, calling into src/ipc/ogc/feasible_region.hpp. It was never enabled anywhere (no caller set ogc_collisions=true), so this is dead-code removal, not a behavior change for any existing user. src/ipc/ogc/ (the standalone OGC implementation) is untouched -- verified via `git status --short src/ipc/ogc/` (clean) and the full [ogc] test suite still passing (19 cases, 4147 assertions). The [high_order_potential*] suite also passes unchanged (22 cases, 36559 assertions).
Reorders `#include <array>` into the correct IncludeCategories group per .clang-format, as flagged by clang-format --dry-run --Werror. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # CMakeLists.txt # src/ipc/barrier/barrier.cpp # src/ipc/barrier/barrier.hpp # src/ipc/config.hpp.in # src/ipc/distance/distance_type.cpp # src/ipc/potentials/potential.cpp # src/ipc/smooth_contact/distance/edge_edge.cpp # src/ipc/smooth_contact/distance/edge_edge.hpp # src/ipc/smooth_contact/distance/point_edge.cpp # src/ipc/smooth_contact/distance/point_edge.hpp # src/ipc/smooth_contact/distance/point_face.cpp # src/ipc/smooth_contact/distance/point_face.hpp # tests/src/tests/barrier/test_barrier.cpp # tests/src/tests/benchmark_eigen.cpp
…gression from the merge - Barrier classes became templates (default T=double) in the ipc-sim/main merge; ClampedLogBarrier/NormalizedClampedLogBarrier call sites in high_order_contact and barrier tests needed <> to keep compiling. - edge_edge_distance_type's EA_EB near-degenerate fallback (normal-vector check + closest-point min) was templated on T but left hardcoded to Eigen::Vector3d; scope it to is_floating_point_v<T> so autodiff/SIMD scalars still compile. - Rename the pre-existing geogram-based test oracle in tests/.../distance_type_reference.hpp (was distance_type_exact.hpp, now shadowed by the new production header of the same name in src/ipc/distance) and give it its own local PARALLEL_THRESHOLD, since production no longer exposes that as a global constant. - edge_edge_distance_type is now unconditionally the thresholded analytic classifier (no more DistanceTypeConfig exact-predicate default), so update two tests whose assumptions predated that: compare against the reference using the same threshold instead of forcing an exact (0) threshold, and accept the edge-interior-vs-vertex classifications that are valid (and distance-preserving) for collinear degenerate edges. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Consistent naming for the two contact formulations:
- ESP (Extremum Sum Potential): src/ipc/esp/, ESP* types, esp_* symbols
- GCP (Geometric Contact Potential): src/ipc/gcp/, GCP* types, gcp_* symbols
Acronyms are capitalized in type names to match the convention already
used in this repo (LBVH, AABB, BVH, IPCWrapper); files, folders and
namespaces stay lowercase, matching src/ipc/ogc/ and namespace ipc::ogc.
Cosmetic only; no functional changes.
Deliberately left untouched:
- The Ferguson2023HighOrderIPC citation and its prose in the docs.
High-Order IPC is a different method from ESP.
- Generic math helpers whose "smooth" is the ordinary mathematical
sense (smooth_heaviside, smooth_clamp*, smooth_mu*, smooth_friction_*),
which are shared with the friction code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follows the high_order_contact -> ESP rename: the arbitrary-point
evaluator is an ESP evaluation at an off-mesh point.
ArbitraryPointPotential -> ArbitraryPointESP
arbitrary_point_potential.{cpp,hpp} -> arbitrary_point_esp.{cpp,hpp}
[arbitrary_point_potential] -> [arbitrary_point_esp]
Also capitalizes the ESP and GCP acronyms in type names (ESPPotential,
ESPParameters, ESPCollisions, GCPPotential, GCPParameters, ...) to match
the convention already used in this repo for LBVH, AABB, BVH and
IPCWrapper. Files, folders and namespaces stay lowercase, matching
src/ipc/ogc/ and namespace ipc::ogc.
Replaces the remaining "HO potential" comments with "ESP potential".
ArbitraryPointBVH is deliberately left alone: it is a general
broad-phase index wrapping ipc::LBVH, not ESP-specific.
Cosmetic only; no functional changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reformat the files flagged by the clang-format 21 check (the version pinned in .github/workflows/clang-format-check.yml). Most are fallout from the ESP/GCP rename: the longer type names (HighOrderContactPotential -> ESPPotential, Esp* -> ESP*) shifted line lengths past the column limit, and the new include paths re-sort (ipc/gcp/... now precedes ipc/geometry/...). A handful of files under esp/collisions/, math.tpp and gcp/distance/mollifier.tpp were already failing before the rename and are fixed here too, since the rename had already touched them. Formatting only; no functional changes. Verified: full build clean and the [esp_potential]/[gcp_potential]/[arbitrary_point_esp]/[smooth_clamp] suites give byte-identical results (45,241 assertions). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # src/ipc/barrier/barrier.cpp # src/ipc/distance/CMakeLists.txt # src/ipc/math/CMakeLists.txt
Three unrelated CI failures on ipc-sim#259: 1. Windows: near/far are macros in windef.h, so MSVC mangled NearFarBarrier's declarations ("error C2059: syntax error: 'const'"). Renamed the two members to near_value/far_value, matching the existing first_derivative_near/far naming. This also broke polyfem's Windows build, which consumes this header. 2. Python (all 3 platforms) and CUDA: the public candidates.hpp included ipc/utils/unordered_map_and_set.hpp, whose own comment documents it as internal because Abseil is a private dependency. The Python bindings include candidates.hpp and do not link Abseil, so they failed with "absl/hash/hash.h: No such file or directory". Moved the nine unordered_map members into a pimpl defined in the .cpp, following the pattern upstream already uses: every other header including that file is a builder/details/internal header unreachable from python/src. The members were only ever used inside candidates.cpp despite being public, so no caller changes. The accessors return {} when the pimpl is null, preserving the previous empty-map behaviour. 3. Windows: geogram 1.9.8's vendored PoissonRecon includes <hash_map>, a pre-standard header removed from current MSVC, and it is compiled unconditionally (no GEOGRAM_WITH_* guard). geogram 1.10.1 drops Hash.h entirely, so bump to it. It no longer pulls predicates.h in transitively via exact_geometry.h, so include it explicitly where PCK::initialize is used. Note IPC_TOOLKIT_WITH_GEOGRAM=OFF is not a viable alternative: it builds, but 4 test cases fail because the *_exact distance-type classifiers fall back to the analytic implementation. Verified locally: [distance-type],[esp_potential],[gcp_potential], [arbitrary_point_esp] pass (39 cases, 4610315 assertions). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
readability-braces-around-statements accounted for 136 of the 200 distinct clang-tidy errors on this branch. Applied via clang-tidy --fix (18.1.8, matching the version CI installs). Braces only: verified the token stream of every changed file is unchanged apart from the added braces. Tests unaffected (51 cases, 4623026 assertions). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes the remaining readability-identifier-naming errors, plus
performance-enum-size, readability-named-parameter and
performance-unnecessary-value-param.
The config sets MemberCase: lower_case and its ignore patterns allow
*leading* underscores ('^(_.*)$'), so trailing-underscore names were
flagged. Upstream's convention for members is a leading m_ prefix
(e.g. CollisionMesh::m_full_rest_positions), so:
mesh_ -> m_mesh (Candidates)
ptr_ / size_ -> m_ptr / m_size (span)
m_A / n_A_rows -> m_a / m_n_a_rows (VertexMatrixView)
use_standard_ -> m_use_standard (DistanceTypeConfig)
_dbar_factor -> dbar_factor_value (public member)
grad_P, P_near -> grad_p, p_near (local struct members)
Trailing-underscore constructor parameters and locals became leading
underscore, matching ParameterIgnoredRegexp/VariableIgnoredRegexp.
Global constants became UPPER_CASE. IntegrationType now has an explicit
std::uint8_t base. Unused override parameters are commented rather than
unnamed, and PointPotential takes ESPParameters by const reference.
span keeps its lowercase name via NOLINTNEXTLINE, since it deliberately
mirrors std::span -- the same escape hatch upstream uses in
default_init_allocator.hpp.
Verified: clang-tidy 18.1.8 (the version CI installs) reports no errors
for the previously failing translation units, and the tests are unchanged
(56 cases, 4623099 assertions).
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
3 participants
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.
Description
Implementation of the SigAsia paper "Extremum Sum Proximity Potential".
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Built and ran the test suite locally on macOS/arm64.
This branch adds the following tests, all passing locally:
New test files
tests/src/tests/potential/test_esp_potential.cpp(19 cases,[esp_potential])ESP potential in 2D and 3D: convergent-quadrature edge-edge limits under
three barriers, gradient/Hessian finite differences, Hessian PSD-ness,
codimensional collisions, adaptive support, NearFarBarrier decomposition.
tests/src/tests/potential/test_arbitrary_point_esp.cpp(6 cases,[arbitrary_point_esp]) Evaluating the ESP at an arbitrary off-mesh pointin 2D/3D: zero beyond dhat, FD gradient/Hessian, and that
evaluate()agrees with
operator()/gradient()/hessian().tests/src/tests/potential/test_smooth_clamp.cpp(12 cases,[smooth_clamp])smooth_clamp01andsmooth_clamp_simplex: anchorvalues, saturation, monotonicity, C0/C1 continuity at knots, FD-vs-analytic
derivatives, and simplex partition-of-unity.
tests/src/tests/utils/test_vertex_matrix_view.cpp(5 cases,[vertex_matrix_view]) Non-owning two-matrix vertex view: concatenationagainst a naive vstack, aliasing semantics, empty-matrix edge case.
New cases in existing files
test_distance_type.cpp: four randomized tests comparing the analyticdistance-type classifiers against a geogram exact-predicate reference
(new
distance_type_reference.hpp), including the near-parallel edge-edge case.test_force_jacobian.cpp: ESP friction force-Jacobian in 2D and 3D(
[friction-esp]).test_barrier.cpp: log barrier derivatives.Test Configuration:
IPC_TOOLKIT_WITH_GEOGRAM=ON,IPC_TOOLKIT_WITH_CUDA=OFFChecklist