Commit fa92933
Use solver-reported accelerations and track runtime gravity in PhysX and OvPhysX IMU/PVA sensors (#7416)
# Description
Three related defects in the IMU and PVA sensors, across PhysX and
OvPhysX. They were originally split across this PR and #7538 (now closed
as superseded); they are combined here because they touch the same
kernels and could not be reviewed or merged independently.
## 1. Accelerations were finite-differenced (#1294)
Both sensors computed linear acceleration by finite-differencing body
velocity between updates. This made the reading depend on the sensor
update period, produced a zero or stale reading on the first update, and
generated large spurious spikes whenever velocities were written
directly (resets, teleports).
**PhysX**: the sensors now read `RigidBodyView.get_accelerations()` and
transport it to the sensor frame as `a_sensor = a_com + α×r + ω×(ω×r)`.
The IMU adds the gravity bias; PVA does not, since PVA reports kinematic
acceleration.
**OvPhysX**: the sensors read accelerations through the
`RIGID_BODY_ACCELERATION` tensor binding with the same transport math.
An earlier revision of this PR guarded that behind `hasattr(TT,
"RIGID_BODY_ACCELERATION")` and fell back to finite differencing,
because the binding had not shipped yet. It has since shipped and the
project pins `ovphysx==0.5.11` exactly, so the fallback was unreachable
on every supported install and has been removed along with its duplicate
kernels and previous-velocity state.
This also aligns PhysX and OvPhysX with Newton, whose PVA already
computed acceleration from `body_qdd` with exactly this transport.
### `get_accelerations()` accuracy
Verified on Isaac Sim 6.0.1 with a constant applied force (expected `a =
F/m`):
| mass | solver acc | expected | rel. err |
|---|---|---|---|
| 1 kg | 0.00199992 | 0.002 | 0.004% |
| 1 g | 2.00004 | 2.0 | 0.002% |
| 1 µg | 2000.0 | 2000.0 | <0.001% |
The historical small-mass inaccuracy that motivated the
finite-difference implementation is no longer present.
## 2. `OvPhysxManager.get_gravity()` returned a stale value
`set_gravity()` authored the new gravity into OvStage and applied it to
the running simulation, but `get_gravity()` read
`SimulationCfg.gravity`, which the setter never touched. Every call
after a live update returned the construction-time value, silently and
permanently:
```
get_gravity before set : (0.0, 0.0, -9.81)
set_gravity called with: (0.0, 0.0, -3.72)
get_gravity after set : (0.0, 0.0, -9.81)
```
The manager now tracks the applied gravity rather than writing back to
`cfg.gravity`. Writing back would have been wrong: both `_call_physx`
and `_call_ovphysx` in `randomize_physics_scene_gravity` resample from
`env.sim.cfg.gravity` as the pristine base, so mutating it would make
successive `"add"`/`"scale"` randomizations compound.
## 3. Sensors snapshotted gravity at initialization
IMU and PVA read gravity once in `_initialize_impl` and baked it into a
static buffer, so gravity randomized at runtime never reached the
accelerometer bias or the projected gravity direction. This affected
**both** backends and was independent of defect 2 — fixing
`get_gravity()` alone would not have changed sensor behavior.
Both sensors now re-read scene gravity on every update, skipping the
work when it is unchanged. This matters more after change 1, not less:
the IMU reading is now literally `solver_a + (−g)`, so a stale `g`
enters as a clean additive bias. Randomizing to Mars gravity made a
resting body read +9.81 instead of +3.72 — a systematic 6.09 m/s² offset
on every sample.
Gravity is scene-wide on both backends, so the bias and the gravity
direction are passed to the kernels **by value** rather than as per-body
buffers. The PhysX recorded launches re-bind the parameter with
`set_param_by_name` when it changes, mirroring how `env_mask` is already
handled. If per-env gravity ever reaches these backends these go back to
arrays, as Newton already does.
## Breaking change
The PhysX PVA sensor's `GRAVITY_VEC_W` proxy array becomes the internal
`_gravity_vec_w` scene-wide vector, matching what the OvPhysX PVA sensor
already called it. It was absent from the data class, the type stubs and
the docs. Consumers wanting this quantity should read
`pva_sensor.data.projected_gravity_b`. **The identically-named attribute
on the asset data classes is untouched.**
## Known remaining gaps (not addressed here)
- `GRAVITY_VEC_W` is still snapshotted at construction in the **asset**
data classes (`rigid_object_data.py`, `rigid_object_collection_data.py`,
`articulation_data.py`) for both backends. Newton fixed its three by
binding `ProxyArray(model.gravity[:world_count])` zero-copy, which is
not possible here: OvPhysX exposes no scene-gravity tensor binding, and
PhysX exposes gravity only as a host-side `Float3`. A fix must use the
same pull-based refresh used here, and first needs a decision on whether
to converge PhysX/OV onto Newton's m/s² convention (they currently store
a normalized direction), which would touch consumers in
`isaaclab_experimental` and `isaaclab_tasks_experimental`.
- Newton's `Imu` applies no gravity bias at all, unlike the PhysX and
OvPhysX IMUs. This looks like a cross-backend behavioral gap rather than
a staleness bug.
Fixes #1294
## Type of change
- Bug fix (non-breaking change which fixes an issue)
- Breaking change (existing functionality will not work without user
modification)
- Documentation update
## Release backport
- [x] <!-- backport-active-release --> Backport this pull request to the
active release branch after it merges into `develop`
## Screenshots
Not applicable — no visual change.
## Checklist
Docker and GPU tests run on demand. Push the commits you want tested,
then
comment `run-ci` on the pull request.
- [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 added a changelog fragment under
`source/<pkg>/changelog.d/` for every touched package (do **not** edit
`CHANGELOG.rst` or bump `extension.toml` — CI handles that)
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
## Testing
All suites run locally, one file at a time:
| Suite | Result |
|---|---|
| `isaaclab_physx` IMU | 11 passed |
| `isaaclab_physx` PVA | 11 passed |
| `isaaclab_physx` recorded-launch | 13 passed |
| `isaaclab_ov` IMU (default / `-k cuda`) | 12 passed, 14 skipped / 12
passed |
| `isaaclab_ov` PVA (default / `-k cuda`) | 13 passed, 15 skipped / 13
passed |
| `isaaclab_ov` physics | 53 passed |
Regression coverage, each verified to fail without its fix:
- `test_set_gravity_writes_and_releases_ovstage_control_resources` —
asserts `get_gravity()` round-trips after a successful `set_gravity()`,
still reports the previously applied value when the OvStage write fails,
and leaves `cfg.gravity` untouched as the randomization base. Without
the fix: `assert (0.0, 0.0, -9.81) == approx((0.0, 0.0, -3.72))`,
reproducing the reported symptom.
- `test_sensor_tracks_runtime_gravity_changes` and
`test_imu_replayed_launch_applies_new_gravity_bias` — a mid-run gravity
change must reach the *replayed* recorded launch, with an explicit guard
that the test is on the recorded path rather than the eager fallback.
- `test_velocity_writes_do_not_produce_spurious_acceleration` (IMU and
PVA) — velocity teleports must not produce an acceleration spike.
Without the fix the PVA case fails with a `99.9998` difference, exactly
the `0.1 / dt = 100 m/s²` finite-difference artifact.
The OvPhysX IMU tests were updated where they encoded finite-difference
artifacts as expected values — most notably `test_gravity_at_rest`,
whose old scenario never had the ball at rest (it fell continuously
while zero velocity was rewritten each step). It now applies a real
support force so the body is genuinely at rest and the accelerometer
reads the bias alone, as a real IMU on a table does.
---------
Co-authored-by: Kelly Guo <kellyg@nvidia.com>
(cherry picked from commit 2d88d27)1 parent 21b60f9 commit fa92933
21 files changed
Lines changed: 597 additions & 368 deletions
File tree
- source
- isaaclab_ov
- changelog.d
- isaaclab_ov
- physics
- sensors
- imu
- pva
- test
- physics
- sensors
- isaaclab_physx
- changelog.d
- isaaclab_physx/sensors
- imu
- pva
- test/sensors
- isaaclab
- changelog.d
- isaaclab/sensors/pva
Lines changed: 6 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
34 | | - | |
35 | | - | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
Lines changed: 9 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
Lines changed: 11 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Lines changed: 15 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
417 | 417 | | |
418 | 418 | | |
419 | 419 | | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
420 | 424 | | |
421 | 425 | | |
422 | 426 | | |
| |||
522 | 526 | | |
523 | 527 | | |
524 | 528 | | |
| 529 | + | |
525 | 530 | | |
526 | 531 | | |
527 | 532 | | |
| |||
688 | 693 | | |
689 | 694 | | |
690 | 695 | | |
691 | | - | |
| 696 | + | |
692 | 697 | | |
693 | 698 | | |
694 | | - | |
| 699 | + | |
| 700 | + | |
695 | 701 | | |
696 | 702 | | |
697 | 703 | | |
698 | 704 | | |
699 | 705 | | |
700 | 706 | | |
701 | | - | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
702 | 710 | | |
703 | 711 | | |
704 | 712 | | |
| |||
745 | 753 | | |
746 | 754 | | |
747 | 755 | | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
748 | 760 | | |
749 | 761 | | |
750 | 762 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
| 49 | + | |
| 50 | + | |
53 | 51 | | |
54 | 52 | | |
55 | 53 | | |
| |||
105 | 103 | | |
106 | 104 | | |
107 | 105 | | |
108 | | - | |
109 | 106 | | |
110 | 107 | | |
111 | 108 | | |
112 | 109 | | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
117 | 110 | | |
118 | 111 | | |
119 | 112 | | |
| |||
149 | 142 | | |
150 | 143 | | |
151 | 144 | | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
156 | 150 | | |
157 | 151 | | |
158 | 152 | | |
| |||
180 | 174 | | |
181 | 175 | | |
182 | 176 | | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
183 | 192 | | |
184 | 193 | | |
185 | 194 | | |
| 195 | + | |
186 | 196 | | |
187 | 197 | | |
188 | 198 | | |
189 | 199 | | |
190 | 200 | | |
| 201 | + | |
191 | 202 | | |
192 | 203 | | |
193 | 204 | | |
| |||
201 | 212 | | |
202 | 213 | | |
203 | 214 | | |
| 215 | + | |
204 | 216 | | |
205 | 217 | | |
206 | 218 | | |
207 | 219 | | |
208 | | - | |
209 | 220 | | |
210 | | - | |
211 | 221 | | |
212 | 222 | | |
213 | 223 | | |
| |||
218 | 228 | | |
219 | 229 | | |
220 | 230 | | |
221 | | - | |
| 231 | + | |
222 | 232 | | |
223 | 233 | | |
224 | 234 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
9 | 30 | | |
10 | 31 | | |
11 | 32 | | |
12 | 33 | | |
13 | 34 | | |
14 | 35 | | |
| 36 | + | |
15 | 37 | | |
16 | 38 | | |
17 | 39 | | |
18 | | - | |
19 | | - | |
| 40 | + | |
20 | 41 | | |
21 | | - | |
22 | | - | |
23 | 42 | | |
24 | 43 | | |
25 | 44 | | |
26 | 45 | | |
27 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
28 | 51 | | |
29 | 52 | | |
30 | 53 | | |
31 | 54 | | |
32 | 55 | | |
| 56 | + | |
33 | 57 | | |
34 | 58 | | |
35 | 59 | | |
36 | | - | |
37 | | - | |
| 60 | + | |
38 | 61 | | |
39 | | - | |
40 | 62 | | |
41 | 63 | | |
42 | 64 | | |
43 | 65 | | |
44 | 66 | | |
45 | 67 | | |
46 | 68 | | |
47 | | - | |
48 | | - | |
| 69 | + | |
| 70 | + | |
49 | 71 | | |
50 | 72 | | |
51 | 73 | | |
52 | 74 | | |
53 | 75 | | |
54 | | - | |
55 | 76 | | |
| 77 | + | |
| 78 | + | |
56 | 79 | | |
57 | 80 | | |
58 | 81 | | |
59 | | - | |
60 | | - | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
61 | 88 | | |
62 | 89 | | |
63 | 90 | | |
64 | 91 | | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
91 | | - | |
0 commit comments