[PERF] Split rigid collision BVH into static + dynamic subsets (RPL multi-depth)#2878
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48736869ad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
4873686 to
8922a3e
Compare
|
Rebased onto current main and fixed a correctness bug! |
8922a3e to
acd9786
Compare
…ulti-depth) Follow-up to Genesis-Embodied-AI#2867. A scene with one moving robot on a large static terrain still rebuilds a single combined collision BVH over every face each step, because the rebuild-skip keys off "all links in the solver are fixed" -- false as soon as the robot moves. The static terrain (the bulk of the faces) is re-fit every step for nothing. This decomposes the rigid solver's collision faces into two compacted BVHs by owning-link fixedness (RaycastContext._partition_collision_faces): - static subset (faces on fixed links: terrain / walls): maybe_static, built once, then skipped + shared across envs (the dominant per-step cost for one robot on a big static terrain). - dynamic subset (faces on movable links: the robot): rebuilt each step, but the rebuild + radix sort now scale with the robot's face count, not the whole scene. The two are cast separately and merged via the existing is_merge path (closest hit wins), so the result is identical to one combined BVH. This is the "multi-depth" decomposition from RPL (arXiv:2602.03002): cast the dynamic robot and static terrain meshes separately and reuse the static acceleration structure across timesteps and environments. Implementation - The split lives in the shared RaycastContext (activate / update), so a Raycaster and a DepthCamera on the same solver share the one static + dynamic BVH pair. Each BVH is built over a compacted face subset: a `face_ids` array maps a BVH leaf slot to the global face index; bvh_ray_cast remaps after reading the morton-code primitive id, and update_aabbs iterates the subset. `n_triangles` in bvh_ray_cast derives from the morton-codes shape (the BVH's own leaf count) instead of the solver-global face count. - The existing maybe_static/needs_rebuild skip and the AABB-derived shared_across_envs test are already per-entry, so they apply to each subset unchanged: the static subset's GEOMETRY subscriber only fires on an explicit set_pos/set_quat (e.g. re-randomized terrain), never on physics-driven robot motion, so it stays skipped + shared while the robot subset rebuilds. - A pure-static or pure-dynamic solver yields a single subset with identity face_ids, i.e. the previous single-BVH behavior -- bit-identical. - kernel_cast_ray (viewer pick) and the viewer plugin thread an identity face_ids over the full mesh. Merge correctness for no_hit_value < max_range - Splitting a robot-on-terrain collision scene turns a former single-BVH cast into a two-BVH merge. The merge overwrites a slot only when it finds a closer hit, so a first-pass miss that stamped `no_hit_value` directly would beat a real hit from a later pass whenever no_hit_value < max_range (e.g. 0/-1) -- previously guarded by a hard `no_hit_value >= max_range` check that would now newly reject those (valid, formerly single-BVH) configs. - write_ray_hit instead seeds an intermediate miss with the `max_range` sentinel (a real hit is always strictly below it, so it loses every distance comparison) and stamps `no_hit_value` only on the final pass, over any slot still at the sentinel. A single-BVH cast is both first and last, so it writes no_hit_value directly -- unchanged. The `no_hit_value >= max_range` check is removed; this also lifts the same restriction from the pre-existing visual+collision merge. Tests - tests/test_sensors.py::test_raycaster_static_dynamic_bvh_split asserts the split structure (one static + one dynamic collision BVH, static shared across envs), the merge reporting the closer of static/dynamic as a movable box enters/leaves a ray, and that the static BVH stays skipped across a dynamic move. n_envs in {0, 2}. - tests/test_sensors.py::test_raycaster_split_merge_no_hit_value_below_max_range casts a ray that misses the static (first-cast) BVH and hits the dynamic one with no_hit_value=-1 < max_range, asserting the real hit is reported and a ray missing both falls back to no_hit_value. n_envs in {0, 2}. - Existing raycaster/lidar/depth suite unchanged (single full-mesh path is the identity-face_ids case; mixed scenes now exercise the two-BVH merge); test_shared_context updated to expect the static+dynamic split (2 entries). Perf (perceptive depth camera, 64x36 rays, 18.5k-face terrain + G1, RTX 3080): 1024 envs: 1037 -> 706 ms/step (1.47x) measured on the pre-refactor branch; win grows with env count x terrain faces. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
acd9786 to
35f491f
Compare
|
🔴 Benchmark Regression Detected ➡️ Report |
35f491f to
1bab0df
Compare
…shared-static-bvh Compose the static/dynamic collision BVH split (Genesis-Embodied-AI#2878) with the shared static BVH grouping: the split's static face subset is now the groupable entry, so its trees collapse across envs even when movable links share the solver. The grouped build kernel and the traversal remap compose through the shared per-face AABB func (face_ids / is_remapped), the grouped allocation pool sizes slots to the subset, and the cast chain carries is_merge / is_last / is_remapped / is_env_major together. The split test asserts the static subset collapses to one tree while the dynamic subset stays per env.
1bab0df to
4767542
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47675421c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
34de7c0 to
8cce7a0
Compare
|
Rebasing onto today's main turned into a near-rewrite: the per-component repack, the distances-only raycaster and the tactile single-tree BVH queries all landed in between, and on top of that I added compile-time remap gating, a combined-BVH contract for the tactile sensors and a pose-based rebuild-skip gate. Rather than ship all of that under your name, I moved the final version to #3078 (with credit back here) and reset this branch to your original commit. |
8cce7a0 to
35f491f
Compare
Description
Follow-up to #2867. That PR added the static-rebuild-skip + shared-BVH-across-envs fast paths, but they only engage when every link in the solver is fixed (
maybe_static = all(link.is_fixed for link in solver.links)). The common RL case — one moving robot on a large static terrain in a single rigid solver — never qualifies: the robot's movable links make the whole solver non-static, so the single combined collision BVH (terrain + robot) is rebuilt over every face each step. The static terrain, which is the bulk of the faces, is re-fit every step for nothing.This PR decomposes the rigid solver's collision faces into two compacted BVHs by owning-link fixedness (
RaycasterSensor._partition_collision_faces):maybe_static, built once, then skipped + shared across envs (the dominant per-step cost for one robot on a big static terrain).The two are cast separately and merged via the existing
is_mergepath (closest hit wins), so the result is identical to one combined BVH. This is the "multi-depth" decomposition from RPL (arXiv:2602.03002): cast the dynamic robot and static terrain meshes separately and reuse the static acceleration structure across timesteps and environments.Implementation
face_idsarray maps a BVH leaf slot → global face index;bvh_ray_castremaps after reading the morton-code primitive id, andupdate_aabbsiterates the subset.n_trianglesinbvh_ray_castnow derives from the morton-codes shape (the BVH's own leaf count) instead of the solver-global face count.maybe_static/needs_rebuildskip and the AABB-derivedshared_across_envstest are already per-entry, so they apply to each subset unchanged: the static subset's GEOMETRY subscriber fires only on an explicitset_pos/set_quat(e.g. re-randomized terrain), never on physics-driven robot motion — so it stays skipped + shared while the robot subset rebuilds.face_ids, i.e. the previous single-BVH behavior — bit-identical.kernel_cast_ray(viewer pick) and the viewer plugin thread an identityface_idsover the full mesh.Motivation and Context
Profiling perceptive whole-body imitation (torso depth camera, 64×36 rays, 18.5k-face terrain + G1-29DoF, RTX 3080): the combined-BVH rebuild dominated
env.stepand #2867's static-skip never engaged because the robot shares the solver. With the split, a moving-robot-on-terrain scene builds 1 static (skipped + shared) + 1 dynamic BVH; depth output is unchanged.Per-step (measured on the pre-refactor branch): 1024 envs 1037 → 706 ms/step (1.47×); the win grows with env count × terrain face count (the eliminated static rebuild scales with both).
How Has This Been Tested?
Environment: single RTX 3080, CUDA backend.
tests/test_sensors.py::test_raycaster_static_dynamic_bvh_split(n_envs ∈ {0, 2}): asserts the split structure (one static + one dynamic collision BVH; staticshared_across_envs), that the merge reports the closer of static/dynamic as a movable box enters/leaves a ray, and that the static BVH stays skipped (needs_rebuildFalse) across a dynamic-only move.face_idscase (bit-identical); mixed scenes now exercise the two-BVH merge.Checklist
test_raycaster_static_dynamic_bvh_split).🤖 Generated with Claude Code