fix(simulator): record contact forces per physics substep, uniformly

[tkevinbest]

Fixes #212 — and two sibling bugs the investigation turned up.

The problem

contact_forces_history was a configurable-length rolling window that meant three different things on the three backends, two of them outright broken. All three are reproduced live on main below.

Backend	Behavior on main	Measured on main (history_length=6, decimation 4)
IsaacSim	Only the newest min(decimation, history_length) slots were ever written; the buffer was allocated at the full configured length.	[716.15, 253.59, 263.82, 305.59, 0.0, 0.0] — slots 4–5 permanently zero
IsaacGym	Rotated per substep but never called refresh_net_contact_force_tensor, which backs contact_forces.	[365.39, 365.39, 365.39, 365.39, 343.71, 343.71] — one frame, duplicated
MuJoCo	Rotated inside refresh_sim_tensors, so a frame was one control step — and the reset path calls that method a second time within the same control step, duplicating a frame for every env whenever any env reset.	—

Permanent zeros are indistinguishable from real zero-force samples, so a consumer that differentiates the history sees a large spurious rising edge once per policy step. contact_sensor_history_length was documented as "frames of contact data retained" with no hint that values above the decimation did nothing.

The fix

Replace it with contact_forces_substep, conforming to the substep-recording paradigm the repo already uses for torques_substep / dof_pos_substep (managers/action/terms/joint_control.py): exactly control_decimation_steps frames, oldest at index 0, one written per physics step from each backend's simulate_at_each_physics_step, with the slot index re-anchored by a FRAME_BEGIN hook.

Because the buffer is fully rewritten every control step, clear_contact_forces_history and the contact_sensor_history_length knob are both retired — the dead tail, the reset staleness and the cross-backend divergence go away by construction rather than by three separate patches.

Recording deliberately does not live in refresh_sim_tensors: that method runs a variable number of times per control step (the reset path calls it again; run_sim and the test harnesses call it per physics step), so nothing time-varying can live there. That invariant is now pinned by a test.

Also drives the IsaacSim contact sensor's update_period from the physics step instead of a hardcoded 5 ms, which only equaled the sim dt at the shipped fps=200.

Reviewing this

Three files carry the design; the rest follows mechanically.

• simulator/shared/contact_substep.py — 29 lines, the whole mechanism.
• simulator/base_simulator/base_simulator.py — the FRAME_BEGIN hook and the two forwarders.
• tests/simulators/contact_substep_assert.py — what "correct" means, stated as eight named properties.

The three backend diffs are each a handful of lines: delete the old rotation, call record_contact_substep after the physics step.

Verification

A new cross-backend harness asserts eight properties: SHAPE, FILLED (every slot written), ORDERED (slot i written at substep i, not before, not again), DISTINCT, AGREES (with contact_forces), REWRITTEN (each control step), RESTARTS (FRAME_BEGIN re-anchors slot 0), STABLE (refresh_sim_tensors never touches the buffer). "Written" is checked by NaN-poisoning rather than looking for non-zero force, since a slot is legitimately zero when the body is airborne.

Green on all four backends, 7 live cases: IsaacSim (1, 4 envs) · IsaacGym (1, 4 envs) · MuJoCo classic (CPU) · mjwarp (1, 4 envs). Plus 11 pure unit tests, 212 no_sim tests, and 82 MuJoCo sim tests.

I ran four deliberate mutations to confirm the harness bites — each fails exactly the check that names it:

Mutation	Caught by
always write slot 0	ORDERED
begin_frame made a no-op	RESTARTS
freeze the frame for a whole step	DISTINCT, AGREES
also record in refresh_sim_tensors (the old MuJoCo placement)	STABLE, ORDERED

RESTARTS and STABLE exist because the first version of the harness passed two of those mutations.

Behavior and cost

• UndesiredContacts now means "peak contact during this control step" on every backend. A 120-iteration WBT A/B at a fixed seed puts it within 1% of before (mean raw 1.339 → 1.325).
• Training is unchanged. 100-iteration locomotion, same seed: IsaacSim learning curves bitwise identical (reward -0.868 → 2.854, episode length 12.727 → 56.780 on both); IsaacGym converges the same (3.081 vs 3.000), with the small divergence expected from the added per-substep refresh that loco terminations read.
• Perf: IsaacSim 326s → 321s (no regression — dropping IsaacLab's now-unused internal history roll offsets the extra reads). IsaacGym 332s → 348s (+4.8%, the added refresh_net_contact_force_tensor). MuJoCo ClassicBackend physics-step rate 7006 → 4881 steps/s (+62 µs/step at ncon=6), since compute_contact_forces now runs per substep — still 2.4× realtime at 200 Hz. That loop is per-contact, so the cost scales with ncon; vectorizing ClassicBackend.compute_contact_forces would remove the concern if it ever matters.

Also verified against the pinned IsaacLab v2.3.0 source: net_forces_w is written unconditionally (only the extra history buffers are gated on history_length, default 0), update_period=0.0 marks the sensor outdated every call, and track_air_time uses elapsed timestamps rather than update_period, so nothing rescales.

Breaking change for downstream configs

contact_sensor_history_length is gone from SimulatorInitConfig, so a downstream registry entry that still sets it will TypeError at import. Saved checkpoint/wandb configs are unaffected. The attribute rename is deliberate too: a consumer reading the old name gets an AttributeError rather than a silently reversed index order feeding a finite difference.