Fix backend asset state invalidation#6150
Conversation
Add cross-backend tests that verify asset writers invalidate cached pose and velocity data. Include changelog fragments and formatter cleanups for the touched backend files.
Address review feedback on the cache-invalidation cleanup: - Add from_link/from_com flags to the Newton reset_pose/reset_velocity helpers so writing a com pose or link velocity no longer clobbers the freshly-written buffer, matching the PhysX and OVPhysX behavior. - Delete a leftover invalidate_fk call in the Newton rigid object collection link-velocity writer that double-fired and ran even when skip_forward was set. - Add the skip_forward parameter to the abstract base writers so the contract matches the concrete backend implementations. - Reword the from_link/from_com docstrings across all backends to state what the flag actually gates, and add kinematic-chain rationale comments to the OVPhysX helpers.
Greptile SummaryThis PR fixes stale cached pose and velocity state after backend asset writers push data into simulation. It refactors all three backends (Newton, PhysX, OVPhysX) so that each data class owns a
Confidence Score: 3/5Not safe to merge as-is — three P1 defects in the Newton backend mean the new skip_forward feature silently misbehaves for RigidObject and RigidObjectCollection callers. The PhysX and OVPhysX backends are correctly implemented and the core data-class refactor is sound. However, the Newton backend has concrete bugs where skip_forward is accepted but not propagated, causing unconditional FK invalidation that defeats the feature's purpose. The regression test also has a coverage gap (only checks leaf index methods, not mask wrappers) so these bugs would not be caught automatically. source/isaaclab_newton/isaaclab_newton/assets/rigid_object/rigid_object.py and source/isaaclab_newton/isaaclab_newton/assets/rigid_object_collection/rigid_object_collection.py require fixes before merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Writer as write_*_to_sim_*(skip_forward)
participant DataClass as ArticulationData / RigidObjectData
participant SimMgr as SimulationManager
Caller->>Writer: "write_*_to_sim_index(data, skip_forward=False)"
Writer->>Writer: kernel launch (writes sim buffers)
alt skip_forward is False
Writer->>DataClass: reset_pose(env_ids) or reset_velocity(env_ids)
DataClass->>DataClass: "stamp dependent buffers timestamp = -1.0"
DataClass->>DataClass: "_fk_timestamp = -1.0"
DataClass->>SimMgr: invalidate_fk(env_ids, articulation_ids)
end
Caller->>DataClass: read body_link_pose_w (lazy access)
DataClass->>DataClass: _ensure_fk_fresh()
alt "_fk_timestamp < _sim_timestamp"
DataClass->>SimMgr: forward() / update_articulations_kinematic()
DataClass->>DataClass: "_fk_timestamp = _sim_timestamp"
end
DataClass-->>Caller: fresh pose data
Reviews (1): Last reviewed commit: "Refine asset cache invalidation helpers ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| env_mask: Environment mask. If None, then all the instances are updated. Shape is (num_instances,). | ||
| skip_forward: Whether to skip the forward pass. Defaults to False. | ||
| """ | ||
| self.write_root_link_pose_to_sim_mask(root_pose=root_pose, env_mask=env_mask) |
There was a problem hiding this comment.
skip_forward is accepted by this method but silently dropped — it is never forwarded to write_root_link_pose_to_sim_mask. Any caller that passes skip_forward=True expecting to defer cache invalidation will have the invalidation run anyway, because the delegate uses its own default of False.
| self.write_root_link_pose_to_sim_mask(root_pose=root_pose, env_mask=env_mask) | |
| self.write_root_link_pose_to_sim_mask(root_pose=root_pose, env_mask=env_mask, skip_forward=skip_forward) |
| SimulationManager.invalidate_fk(env_ids=env_ids, articulation_ids=self._root_view.articulation_ids) | ||
| # Invalidate dependent timestamps. The link velocities were just written, so they must not be | ||
| # invalidated. ``reset_velocity`` already triggers the simulation-side FK invalidation. | ||
| if not skip_forward: |
There was a problem hiding this comment.
SimulationManager.invalidate_fk() call that was not removed when timestamp invalidation was refactored into reset_velocity. reset_velocity already calls SimulationManager.invalidate_fk internally, so this produces a redundant double-call when skip_forward=False, and — more critically — still fires when skip_forward=True, defeating the entire purpose of the flag for this method. All other pose/velocity writers in this file had the corresponding call removed or moved inside reset_pose/reset_velocity; this one was missed.
| self.write_body_link_pose_to_sim_index( | ||
| body_poses=body_poses, env_ids=env_ids, body_ids=body_ids, full_data=True | ||
| ) | ||
| if not skip_forward: | ||
| self.data.reset_pose(env_ids=env_ids) |
There was a problem hiding this comment.
skip_forward is not propagated to write_body_link_pose_to_sim_index, so the index method always runs with skip_forward=False and calls reset_pose unconditionally. The outer if not skip_forward: self.data.reset_pose(...) then calls it a second time when skip_forward=False, and is a no-op when skip_forward=True since the inner call already fired. The same pattern exists in write_body_com_pose_to_sim_mask, write_body_com_velocity_to_sim_mask, and write_body_link_velocity_to_sim_mask.
1 participant
Description
Fix stale cached pose and velocity state after backend asset state writers push data into simulation.
This updates the Newton, PhysX, and OV PhysX backend asset writers to invalidate the affected cached
data through shared data-class reset helpers, and adds regression coverage that checks the invalidation
paths across articulations, rigid objects, and rigid object collections.
Fixes # (issue): N/A
Type of change
Screenshots
N/A
Validation
./isaaclab.sh -p -m pytest source/isaaclab/test/assets/test_backend_asset_invalidation.py./isaaclab.sh -f(all hooks passed exceptcheck-git-lfs-pointers, which failed becausegit-lfsis not installed locally)SKIP=check-git-lfs-pointers ./isaaclab.sh -fChecklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there