Implement Fabric-aware get/set_local_poses via indexedfabricarray#2
Open
pv-nvidia wants to merge 11 commits intofix/fabric-prepare-for-reusefrom
Open
Implement Fabric-aware get/set_local_poses via indexedfabricarray#2pv-nvidia wants to merge 11 commits intofix/fabric-prepare-for-reusefrom
pv-nvidia wants to merge 11 commits intofix/fabric-prepare-for-reusefrom
Conversation
pv-nvidia
changed the title
…m#5473) # Description <!-- Thank you for your interest in sending a pull request. Please make sure to check the contribution guidelines. Link: https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html 💡 Please try to keep PRs small and focused. Large PRs are harder to review and merge. --> Extend debug Visualization Markers, which are supported in the Kit Visualizer, to the Newton Visualizers. These Visualization Markers are various shapes and models which can be added to envs for debugging / showing extra information. Also added filtering for partial visualization (when we filtered which envs are shown the in the visualizer, we also filter the markers) For general USD mesh marker support in Newton, a followup PR will be required, once a Newton API for general USD -> Newton Mesh conversion is added (see newton-physics/newton#2667) Checked velocity arrows, dexcubes, raycasts, frames, goal markers <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - New feature (non-breaking change which adds functionality) - Documentation update ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Signed-off-by: matthewtrepte <mtrepte@nvidia.com>
# Description - Increase the CI startup-hang grace period from 45s to 120s so slow but valid Kit startup is not killed prematurely. - Make `SurfaceGripper` fail fast on non-CPU simulation backends before loading the surface gripper extension. - Skip the CI-only `SurfaceGripperView` CPU initialization path that can deadlock, while keeping CUDA fail-fast coverage. ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task -->
…isaac-sim#5478) Follow-up to isaac-sim#5434 (fragment-based changelog system). Two contributor-facing references still pointed at the old "edit CHANGELOG.rst directly" workflow: - **`docs/source/refs/contributing.rst`** — *Maintaining a changelog and extension.toml* section described per-version editing of CHANGELOG.rst with manual SemVer bumps. - **`.github/PULL_REQUEST_TEMPLATE.md`** — checklist asked contributors to update the changelog and bump extension.toml directly. Replaced only the parts that talk about direct editing; section/style guidance (Added/Changed/Deprecated/Removed/Fixed, past tense, the sample bullets themselves) stays intact. ## Test plan - [x] Pre-commit clean - [ ] Verify Build Latest Docs CI step renders the new section correctly cc @kellyguo11 — addresses the doc gaps flagged after isaac-sim#5434 merged.
# Description Mark all RTX-based rendering test cases flaky until they can produce deterministic low-res camera outputs that pass golden image testing on every CI run. Fixes # (issue) <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> ## Type of change - Test change ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Signed-off-by: HuiDong Chen <huidongc@nvidia.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…ic_write (isaac-sim#5380) ## Summary Replace the `sync_usd_on_fabric_write` workaround in `FabricFrameView` with proper `PrepareForReuse()` calls on the Fabric `PrimSelection`. This tells the renderer (FSD/Storm) that Fabric data has changed, so the next rendered frame reflects updated transforms — eliminating the need to copy Fabric writes back to USD. ## Motivation The existing `sync_usd_on_fabric_write` flag worked by mirroring every Fabric write back to USD, which defeated the performance benefits of Fabric. With `PrepareForReuse()`, the rendering pipeline is properly notified of Fabric data changes without any USD writeback. Additionally, the old code incorrectly fell back to USD for CPU devices — Warp handles CPU Fabric buffers correctly, so the fallback was unnecessary. This addresses two of the issues raised in @pbarejko Piotr's review of PR isaac-sim#4923: - **Issue #1** (USD write-back): Fabric writes no longer sync back to USD - **Issue isaac-sim#4** (PrepareForReuse): Renderer notification via `PrepareForReuse()` instead of USD writeback ## Changes ### Core (FabricFrameView) - Call `_prepare_for_reuse()` in write paths (`set_world_poses`, `set_scales`) to notify the renderer - Remove `sync_usd_on_fabric_write` parameter (accepted via `**kwargs` for backward compat) - Remove incorrect CPU/device fallback warnings — Warp handles CPU Fabric buffers correctly - Add `_rebuild_fabric_arrays()` for topology change recovery when `PrepareForReuse()` returns True, with assertion guarding the prim-count invariant ### Camera - Remove `sync_usd_on_fabric_write=True` from FrameView construction in `camera.py` ## Benchmark Results 1024 prims, 50 iterations, NVIDIA L40 GPU: | Operation | USD (ms) | Fabric (ms) | Speedup | |---|---|---|---| | Get World Poses | 14.71 | 0.07 | **203x** | | Set World Poses | 40.75 | 0.16 | **259x** | | Interleaved Set→Get | 55.90 | 0.24 | **232x** | | Get Local Poses | 11.08 | 11.12 | 1.0x | | Set Local Poses | 16.14 | 16.28 | 1.0x | Local poses fall back to USD (expected — Fabric only accelerates world poses via `omni:fabric:worldMatrix`). ## Tests Added | Test | What it validates | |------|------------------| | `test_camera_pose_update_reflected_in_render` | Camera pose changes propagate to rendered depth (close vs far) for CPU/GPU, tiled/non-tiled | | `test_fabric_set_world_does_not_write_back_to_usd` | Fabric writes stay in Fabric, USD prim unchanged | | `test_set_world_updates_local` (xfail) | Documents Issue isaac-sim#5: `set_world_poses` doesn't update local pose in Fabric mode | ## Test Results | Test Suite | Passed | Skipped | Xfailed | Total | |---|---|---|---|---| | Fabric contract tests (`test_views_xform_prim_fabric.py`) | 17 | 16 | 1 | 34 | | USD contract tests (`test_views_xform_prim.py`) | 45 | 0 | 0 | 45 | | Camera render test (`test_tiled_camera.py`) | 8 | 0 | 0 | 8 | ## Type of change - Performance improvement (removes redundant USD writeback on Fabric operations) ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there *No doc changes needed (parameter wasn't referenced in any docs)*
…tation (isaac-sim#5506) # Description This PR changes the PyTorch3d installation command in the locomanipulation SDG policy training / rollout to use git and install pytorch3d from source. Fixes # (issue) NV bug 6115836 ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task -->
Replaces the earlier Python-based parent-loop implementation (which was
correct but ~3× slower than USD for local poses) with a fully GPU-side
Fabric path that follows the bareya/pbarejko/camera-update prototype:
* Three persistent ``PrimSelection`` instances differing only in
per-attribute access mode — one each for {trans_ro, world_rw, local_rw}.
* Path-based view → fabric index mapping computed once from
``selection.GetPaths()`` and stored as a Warp ``int32`` array. No
custom prim attributes are written to the stage.
* All transform reads and writes go through ``wp.indexedfabricarray``,
so the kernels just dereference ``ifa[view_index]`` instead of taking
a separate mapping argument.
* Stage-level ``IFabricHierarchy`` cache and dirty-stages set so multiple
``FabricFrameView`` instances on the same stage share state.
World ↔ local consistency is preserved through Warp kernels that run on
the affected write paths:
* ``set_local_poses`` writes ``omni:fabric:localMatrix`` directly via the
compose kernel, then a second kernel recomputes child worldMatrix from
``parent_world * child_local`` so the next ``get_world_poses`` read is
consistent. ``IFabricHierarchy.update_world_xforms()`` is *not* used
for this — in practice it re-reads USD's authored xformOps and would
overwrite the matrices we just authored.
* ``set_world_poses`` mirrors the above, recomputing
``child_local = inv(parent_world) * child_world`` after the write.
Two new public Warp kernels in ``isaaclab.utils.warp.fabric``:
* ``decompose_indexed_fabric_transforms`` /
``compose_indexed_fabric_transforms`` — indexed-array variants of the
existing decompose/compose kernels.
* ``update_indexed_local_matrix_from_world`` /
``update_indexed_world_matrix_from_local`` — propagate one direction
using a parent indexed fabric array. Implemented directly in storage
convention (``local = world * inv(parent)``,
``world = local * parent``) — the four transposes the math-convention
form would imply cancel out under ``(A·B)^T = B^T·A^T``.
Benchmark (1024 prims, 50 iterations, RTX A6000):
Operation USD (ms) Fabric (ms) Speedup
Get World Poses 12.33 0.044 282×
Set World Poses 27.98 0.117 240×
Interleaved Set→Get 41.34 0.160 258×
Get Local Poses 6.04 0.037 162×
Set Local Poses 8.54 0.053 162×
Local-pose ops went from ~3× slower than USD (in the earlier
torch-based parent-loop implementation) to ~160× faster, with the
new Fabric-side localMatrix authoring keeping ``test_set_world_updates_local``
passing without an xfail override.
Tests: 41 passed in the Fabric backend's contract + new coverage for
the Fabric-native ``set_local_poses``, ``get_scales``, and topology
rebuild paths.
Drops the no-longer-needed ``cd571d482`` Python-loop attempt.
github-actions
Bot
added
documentation
Three review fixes on the indexedfabricarray refactor: * set_scales wrote the new scale into worldMatrix but never refreshed localMatrix, so a subsequent get_local_poses returned the stale scale. Call _sync_local_from_world after the world write, matching set_world_poses. * The view->fabric mapping was stored in a single shared _fabric_indices field that each accessor overwrote from its own selection's GetPaths() ordering. Selections do not guarantee a shared path ordering, so this was brittle and hard to reason about. Cache the mapping per selection (_trans_ro_fabric_indices, _world_rw_fabric_indices, _local_rw_fabric_indices) and pass it explicitly to _build_indexed_array. The three trans_ro accessors now share a _rebuild_trans_ro_arrays helper. * Update two test comments that referenced the removed _fabric_usd_sync_done attribute to point at the lazy _initialize_fabric() call instead.
The chained set_world_poses -> get_world_poses round-trip in test_fabric_rebuild_after_topology_change goes through Warp's float32 SRT compose/decompose, which accumulates a few ULP of drift. At the test's position magnitudes (~4-6), one float32 ULP is ~4.77e-7, so the prior atol of 1e-7 demanded sub-ULP agreement and was sensitive to GPU/codegen variation -- it passed locally on the A6000 but flaked in CI. 1e-5 corresponds to roughly 20 ULP at those magnitudes: tight enough to catch any real bug (a wrong index or stale read would be at least ~1e-3 off given the test setup) and consistent with the shared contract harness in frame_view_contract_utils.py, which already documents and uses ATOL = 1e-5 for compose/decompose-through-float32 checks.
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
6 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
Replaces isaac-sim#5381
Adopts the prototype design from bareya/pbarejko/camera-update: all transform reads and writes go through
wp.indexedfabricarray, the view→fabric mapping is path-based (no custom prim attributes), andFabricFrameViewreads/writesomni:fabric:localMatrixdirectly.Previously
get_local_posesdelegated to USD even when Fabric held the source-of-truth world matrices, so afterset_world_poses()the nextget_local_poses()returned stale values — the reasontest_set_world_updates_localwas markedxfail(strict=True). An earlier attempt fixed this by composinglocal = inv(parent_world) * child_worldin torch using a Python-level USD parent loop, which was correct but 3× slower than USD. This PR replaces that with two Warp kernels operating on Fabric matrices via a parent indexed fabric array, eliminating the Python bottleneck.Implementation summary
PrimSelectioninstances (trans_ro,world_rw,local_rw) differing only in per-attribute access mode.selection.GetPaths(); noisaaclab:view_index:HASHprim attributes written anywhere.wp.indexedfabricarrayeverywhere: kernels dereferenceifa[view_index]instead of taking a separate mapping argument.IFabricHierarchycache + dirty-stages set: multipleFabricFrameViewinstances on the same stage share one hierarchy lookup.set_local_posesmarks the stage dirty; the next world read fireschild_world = parent * child_localper child. Mirror kernel runs afterset_world_posesto recomputechild_local = inv(parent) * child_world.isaaclab.utils.warp.fabric:decompose_indexed_fabric_transformsandcompose_indexed_fabric_transforms, plus propagation kernelsupdate_indexed_local_matrix_from_worldandupdate_indexed_world_matrix_from_local.The shared contract test
test_set_world_updates_localno longer needs thexfailoverride and passes for bothcpuandcuda:0. Three additional tests cover the new paths:test_set_local_via_fabric_path,test_get_scales_fabric_path,test_prepare_for_reuse_detects_topology_change. Local run of the full fabric test suite: 41 passed, 0 failed.Benchmark results
1024 prims, 50 iterations, NVIDIA RTX A6000.
Reproduce with: