Skip to content

Commit e0376eb

Browse files
Fix lazy PhysX IMU and PVA acceleration (#6439)
## Summary - compute PhysX IMU and PVA finite-difference acceleration from the elapsed time between sensor samples - keep timestamp inputs stable inside the recorded Warp launches from #6390 - remove mutable scalar inverse-dt launch parameters - add lazy-read and nonzero-update-period regressions for IMU linear acceleration and PVA linear/angular acceleration This PR is stacked on #6390. Until #6390 merges, its commits are included in this comparison against develop. ## Root cause IMU and PVA cached velocity only when their buffers were recomputed, but divided the next velocity delta by the most recent physics-step dt. With four lazy physics updates or an update period of four physics steps, both sensors reported 4x acceleration. The kernels now derive a per-environment interval from timestamp minus timestamp_last_update. ## Validation - Unmodified #6390 base: 4 regressions failed with the expected 4x acceleration error - Fixed targeted regressions: 4 passed - Complete PhysX IMU suite: 11 passed - Complete PhysX PVA suite: 11 passed - Final pre-commit hooks: passed - Existing recorded-launch assertions remain enabled and pass ## Performance Benchmarking is deferred until the test machine is connected to AC power. The change removes scalar launch-parameter updates and retains the recorded-launch path; this PR makes no performance claims. --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com>
1 parent 68d7f93 commit e0376eb

8 files changed

Lines changed: 101 additions & 42 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fixed
2+
^^^^^
3+
4+
* Fixed PhysX IMU and PVA acceleration for lazy reads and nonzero update periods.

source/isaaclab_physx/isaaclab_physx/sensors/imu/imu.py

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ def __init__(self, cfg: ImuCfg):
7070
self._raw_coms: wp.array | None = None
7171
self._update_cmd: wp.Launch | None = None
7272
self._update_env_mask: wp.array | None = None
73-
self._update_inv_dt: float | None = None
7473
self._use_recorded_launch: bool = False
7574

7675
def __str__(self) -> str:
@@ -115,10 +114,6 @@ def reset(self, env_ids: Sequence[int] | None = None, env_mask: wp.array | None
115114
device=self._device,
116115
)
117116

118-
def update(self, dt: float, force_recompute: bool = False):
119-
self._dt = dt
120-
super().update(dt, force_recompute)
121-
122117
"""
123118
Implementation.
124119
"""
@@ -187,13 +182,11 @@ def _update_buffers_impl(self, env_mask: wp.array | None = None):
187182
)
188183
wp.copy(self._coms_buffer, self._raw_coms)
189184

190-
inv_dt = 1.0 / self._dt
191185
if self._use_recorded_launch:
192186
if self._update_cmd is None:
193187
try:
194-
self._update_cmd = self._launch_update(env_mask, inv_dt, record_cmd=True)
188+
self._update_cmd = self._launch_update(env_mask, record_cmd=True)
195189
self._update_env_mask = env_mask
196-
self._update_inv_dt = inv_dt
197190
except Exception as exc:
198191
self._use_recorded_launch = False
199192
logger.warning(
@@ -204,15 +197,12 @@ def _update_buffers_impl(self, env_mask: wp.array | None = None):
204197
if env_mask is not self._update_env_mask:
205198
self._update_cmd.set_param_by_name("env_mask", env_mask)
206199
self._update_env_mask = env_mask
207-
if inv_dt != self._update_inv_dt:
208-
self._update_cmd.set_param_by_name("inv_dt", inv_dt)
209-
self._update_inv_dt = inv_dt
210200
self._update_cmd.launch()
211201
return
212202

213-
self._launch_update(env_mask, inv_dt)
203+
self._launch_update(env_mask)
214204

215-
def _launch_update(self, env_mask: wp.array, inv_dt: float, record_cmd: bool = False) -> wp.Launch | None:
205+
def _launch_update(self, env_mask: wp.array, record_cmd: bool = False) -> wp.Launch | None:
216206
"""Launch or record the kernel that updates the IMU data."""
217207

218208
return wp.launch(
@@ -226,8 +216,8 @@ def _launch_update(self, env_mask: wp.array, inv_dt: float, record_cmd: bool = F
226216
self._offset_pos_b,
227217
self._offset_quat_b,
228218
self._gravity_bias_w,
229-
inv_dt,
230219
self._timestamp,
220+
self._timestamp_last_update,
231221
self._prev_lin_vel_w,
232222
self._data._ang_vel_b,
233223
self._data._lin_acc_b,
@@ -258,4 +248,3 @@ def _invalidate_initialize_callback(self, event):
258248
self._raw_coms = None
259249
self._update_cmd = None
260250
self._update_env_mask = None
261-
self._update_inv_dt = None

source/isaaclab_physx/isaaclab_physx/sensors/imu/kernels.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ def imu_update_kernel(
1616
offset_pos_b: wp.array(dtype=wp.vec3f),
1717
offset_quat_b: wp.array(dtype=wp.quatf),
1818
gravity_bias_w: wp.array(dtype=wp.vec3f),
19-
inv_dt: wp.float32,
2019
timestamp: wp.array(dtype=wp.float32),
20+
timestamp_last_update: wp.array(dtype=wp.float32),
2121
# inputs / outputs
2222
prev_lin_vel_w: wp.array(dtype=wp.vec3f),
2323
# outputs
@@ -34,8 +34,8 @@ def imu_update_kernel(
3434
offset_pos_b: Offset positions of the sensors.
3535
offset_quat_b: Offset quaternions of the sensors.
3636
gravity_bias_w: Gravity bias in the world frame.
37-
inv_dt: Inverse of the time step.
3837
timestamp: Timestamp of the environment.
38+
timestamp_last_update: Timestamp of the previous sensor sample.
3939
prev_lin_vel_w: Previous linear velocity in the world frame.
4040
out_ang_vel_b: Output angular velocity in the body frame.
4141
out_lin_acc_b: Output linear acceleration in the body frame.
@@ -49,6 +49,11 @@ def imu_update_kernel(
4949
if timestamp[idx] == 0.0:
5050
return
5151

52+
elapsed_time = timestamp[idx] - timestamp_last_update[idx]
53+
if elapsed_time <= 0.0:
54+
return
55+
inv_dt = 1.0 / elapsed_time
56+
5257
body_quat = wp.transform_get_rotation(transforms[idx])
5358

5459
lin_vel_w = wp.spatial_top(velocities[idx])

source/isaaclab_physx/isaaclab_physx/sensors/pva/kernels.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ def pva_update_kernel(
1616
offset_pos_b: wp.array(dtype=wp.vec3f),
1717
offset_quat_b: wp.array(dtype=wp.quatf),
1818
gravity_vec_w: wp.array(dtype=wp.vec3f),
19-
inv_dt: wp.float32,
2019
timestamp: wp.array(dtype=wp.float32),
20+
timestamp_last_update: wp.array(dtype=wp.float32),
2121
# inputs / outputs
2222
prev_lin_vel_w: wp.array(dtype=wp.vec3f),
2323
prev_ang_vel_w: wp.array(dtype=wp.vec3f),
@@ -40,8 +40,8 @@ def pva_update_kernel(
4040
offset_pos_b: Offset positions of the sensors.
4141
offset_quat_b: Offset quaternions of the sensors.
4242
gravity_vec_w: Gravity direction unit vector in the world frame.
43-
inv_dt: Inverse of the time step.
4443
timestamp: Timestamp of the environment.
44+
timestamp_last_update: Timestamp of the previous sensor sample.
4545
prev_lin_vel_w: Previous linear velocity in the world frame.
4646
prev_ang_vel_w: Previous angular velocity in the world frame.
4747
out_pos_w: Output position in the world frame.
@@ -61,6 +61,11 @@ def pva_update_kernel(
6161
if timestamp[idx] == 0.0:
6262
return
6363

64+
elapsed_time = timestamp[idx] - timestamp_last_update[idx]
65+
if elapsed_time <= 0.0:
66+
return
67+
inv_dt = 1.0 / elapsed_time
68+
6469
body_pos = wp.transform_get_translation(transforms[idx])
6570
body_quat = wp.transform_get_rotation(transforms[idx])
6671

source/isaaclab_physx/isaaclab_physx/sensors/pva/pva.py

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ def __init__(self, cfg: PvaCfg):
8383
self._raw_coms: wp.array | None = None
8484
self._update_cmd: wp.Launch | None = None
8585
self._update_env_mask: wp.array | None = None
86-
self._update_inv_dt: float | None = None
8786
self._use_recorded_launch: bool = False
8887

8988
def __str__(self) -> str:
@@ -138,12 +137,6 @@ def reset(self, env_ids: Sequence[int] | None = None, env_mask: wp.array | None
138137
device=self._device,
139138
)
140139

141-
def update(self, dt: float, force_recompute: bool = False):
142-
# save timestamp
143-
self._dt = dt
144-
# execute updating
145-
super().update(dt, force_recompute)
146-
147140
"""
148141
Implementation.
149142
"""
@@ -220,13 +213,11 @@ def _update_buffers_impl(self, env_mask: wp.array | None = None):
220213
)
221214
wp.copy(self._coms_buffer, self._raw_coms)
222215

223-
inv_dt = 1.0 / self._dt
224216
if self._use_recorded_launch:
225217
if self._update_cmd is None:
226218
try:
227-
self._update_cmd = self._launch_update(env_mask, inv_dt, record_cmd=True)
219+
self._update_cmd = self._launch_update(env_mask, record_cmd=True)
228220
self._update_env_mask = env_mask
229-
self._update_inv_dt = inv_dt
230221
except Exception as exc:
231222
self._use_recorded_launch = False
232223
logger.warning(
@@ -237,15 +228,12 @@ def _update_buffers_impl(self, env_mask: wp.array | None = None):
237228
if env_mask is not self._update_env_mask:
238229
self._update_cmd.set_param_by_name("env_mask", env_mask)
239230
self._update_env_mask = env_mask
240-
if inv_dt != self._update_inv_dt:
241-
self._update_cmd.set_param_by_name("inv_dt", inv_dt)
242-
self._update_inv_dt = inv_dt
243231
self._update_cmd.launch()
244232
return
245233

246-
self._launch_update(env_mask, inv_dt)
234+
self._launch_update(env_mask)
247235

248-
def _launch_update(self, env_mask: wp.array, inv_dt: float, record_cmd: bool = False) -> wp.Launch | None:
236+
def _launch_update(self, env_mask: wp.array, record_cmd: bool = False) -> wp.Launch | None:
249237
"""Launch or record the kernel that updates the PVA data."""
250238

251239
return wp.launch(
@@ -259,8 +247,8 @@ def _launch_update(self, env_mask: wp.array, inv_dt: float, record_cmd: bool = F
259247
self._offset_pos_b,
260248
self._offset_quat_b,
261249
self.GRAVITY_VEC_W,
262-
inv_dt,
263250
self._timestamp,
251+
self._timestamp_last_update,
264252
self._prev_lin_vel_w,
265253
self._prev_ang_vel_w,
266254
self._data._pos_w,
@@ -303,7 +291,6 @@ def _invalidate_initialize_callback(self, event):
303291
self._raw_coms = None
304292
self._update_cmd = None
305293
self._update_env_mask = None
306-
self._update_inv_dt = None
307294

308295
def _set_debug_vis_impl(self, debug_vis: bool):
309296
# set visibility of markers

source/isaaclab_physx/test/sensors/test_imu.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,3 +553,35 @@ def test_sensor_print(setup_sim):
553553
sensor = scene.sensors["imu_ball"]
554554
# print info
555555
print(sensor)
556+
557+
558+
@pytest.mark.parametrize("access_mode", ("lazy_read", "update_period"))
559+
def test_acceleration_uses_elapsed_sensor_time(setup_sim, access_mode):
560+
"""Acceleration uses the elapsed time between sensor samples."""
561+
sim, scene = setup_sim
562+
dt = sim.get_physics_dt()
563+
body = scene.rigid_objects["balls"]
564+
sensor = scene.sensors["imu_ball"]
565+
velocity = torch.zeros((scene.num_envs, 6), dtype=torch.float32, device=scene.device)
566+
567+
body.write_root_velocity_to_sim_index(root_velocity=velocity)
568+
scene.write_data_to_sim()
569+
sim.step()
570+
scene.update(dt)
571+
_ = sensor.data
572+
573+
scene.cfg.lazy_sensor_update = True
574+
if access_mode == "update_period":
575+
sensor.cfg.update_period = 4 * dt
576+
577+
for step in range(4):
578+
velocity[:, 0] = 0.1 * (step + 1)
579+
body.write_root_velocity_to_sim_index(root_velocity=velocity)
580+
scene.write_data_to_sim()
581+
sim.step()
582+
scene.update(dt)
583+
if access_mode == "update_period":
584+
_ = sensor.data
585+
586+
expected = torch.full((scene.num_envs,), 0.1 / dt, device=scene.device)
587+
torch.testing.assert_close(sensor.data.lin_acc_b.torch[:, 0], expected)

source/isaaclab_physx/test/sensors/test_imu_pva_recorded_launch.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ def _make_sensor(sensor_type: str, use_recorded_launch: bool = True):
108108
sensor._device = device
109109
sensor._num_envs = 2
110110
sensor._view = rigid_view
111-
sensor._dt = 0.5
112111
sensor._timestamp = wp.ones(2, dtype=wp.float32, device=device)
112+
sensor._timestamp_last_update = wp.full(2, value=0.5, dtype=wp.float32, device=device)
113113
sensor._offset_pos_b = wp.zeros(2, dtype=wp.vec3f, device=device)
114114
sensor._offset_quat_b = wp.array(
115115
[wp.quatf(0.0, 0.0, 0.0, 1.0), wp.quatf(0.0, 0.0, 0.0, 1.0)], dtype=wp.quatf, device=device
@@ -121,7 +121,6 @@ def _make_sensor(sensor_type: str, use_recorded_launch: bool = True):
121121
sensor._raw_coms = None
122122
sensor._update_cmd = None
123123
sensor._update_env_mask = None
124-
sensor._update_inv_dt = None
125124
sensor._use_recorded_launch = use_recorded_launch
126125
sensor._initialize_handle = None
127126
sensor._invalidate_initialize_handle = None
@@ -158,7 +157,7 @@ def test_sensor_caches_physx_typed_views(sensor_type):
158157

159158
@pytest.mark.parametrize("sensor_type", ["imu", "pva"])
160159
def test_sensor_records_and_replays_changed_runtime_inputs(sensor_type):
161-
"""Replay should observe refreshed buffers, a new mask, and a changed timestep."""
160+
"""Replay should observe refreshed buffers, a new mask, and a changed sample interval."""
162161
sensor, rigid_view, velocities_torch, env_mask = _make_sensor(sensor_type)
163162

164163
sensor._update_buffers_impl(env_mask)
@@ -172,7 +171,8 @@ def test_sensor_records_and_replays_changed_runtime_inputs(sensor_type):
172171
)
173172

174173
velocities_torch[:, 0] = torch.tensor([3.0, 5.0], device=sensor.device)
175-
sensor._dt = 0.25
174+
wp.to_torch(sensor._timestamp_last_update).fill_(1.0)
175+
wp.to_torch(sensor._timestamp).fill_(1.25)
176176
changed_env_mask = wp.array([False, True], dtype=wp.bool, device=sensor.device)
177177
sensor._update_buffers_impl(changed_env_mask)
178178
wp.synchronize_device(sensor.device)
@@ -219,7 +219,6 @@ def test_sensor_invalidation_drops_cached_launch_state(monkeypatch, sensor_type)
219219
sensor._raw_coms = object()
220220
sensor._update_cmd = object()
221221
sensor._update_env_mask = object()
222-
sensor._update_inv_dt = 1.0
223222
base_cls = BaseImu if sensor_type == "imu" else BasePva
224223
monkeypatch.setattr(base_cls, "_invalidate_initialize_callback", lambda self, event: None)
225224

@@ -231,4 +230,3 @@ def test_sensor_invalidation_drops_cached_launch_state(monkeypatch, sensor_type)
231230
assert sensor._raw_coms is None
232231
assert sensor._update_cmd is None
233232
assert sensor._update_env_mask is None
234-
assert sensor._update_inv_dt is None

source/isaaclab_physx/test/sensors/test_pva.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,3 +826,42 @@ def test_sensor_print(setup_sim):
826826
sensor = scene.sensors["pva_ball"]
827827
# print info
828828
print(sensor)
829+
830+
831+
@pytest.mark.parametrize("access_mode", ("lazy_read", "update_period"))
832+
def test_acceleration_uses_elapsed_sensor_time(setup_sim, access_mode):
833+
"""Linear and angular acceleration use the elapsed time between sensor samples."""
834+
sim, scene = setup_sim
835+
dt = sim.get_physics_dt()
836+
body = scene.rigid_objects["balls"]
837+
sensor = scene.sensors["pva_ball"]
838+
velocity = torch.zeros((scene.num_envs, 6), dtype=torch.float32, device=scene.device)
839+
840+
body.write_root_velocity_to_sim_index(root_velocity=velocity)
841+
scene.write_data_to_sim()
842+
sim.step()
843+
scene.update(dt)
844+
_ = sensor.data
845+
846+
scene.cfg.lazy_sensor_update = True
847+
if access_mode == "update_period":
848+
sensor.cfg.update_period = 4 * dt
849+
850+
for step in range(4):
851+
velocity[:, 0] = 0.1 * (step + 1)
852+
velocity[:, 5] = 0.2 * (step + 1)
853+
body.write_root_velocity_to_sim_index(root_velocity=velocity)
854+
scene.write_data_to_sim()
855+
sim.step()
856+
scene.update(dt)
857+
if access_mode == "update_period":
858+
_ = sensor.data
859+
860+
expected_lin_acc = torch.full((scene.num_envs,), 0.1 / dt, device=scene.device)
861+
expected_ang_acc = torch.full((scene.num_envs,), 0.2 / dt, device=scene.device)
862+
torch.testing.assert_close(
863+
torch.linalg.vector_norm(sensor.data.lin_acc_b.torch, dim=-1), expected_lin_acc, rtol=1e-4, atol=1e-3
864+
)
865+
torch.testing.assert_close(
866+
torch.linalg.vector_norm(sensor.data.ang_acc_b.torch, dim=-1), expected_ang_acc, rtol=1e-4, atol=1e-3
867+
)

0 commit comments

Comments
 (0)