Skip to content

perf: skip substep callbacks for invariant actions - #1277

Merged
TATP-233 merged 1 commit into
dev/issue-1042-manager-based-apifrom
perf/issue-1276-state-independent-actions
Aug 24, 2026
Merged

perf: skip substep callbacks for invariant actions#1277
TATP-233 merged 1 commit into
dev/issue-1042-manager-based-apifrom
perf/issue-1276-state-independent-actions

Conversation

@TATP-233

Copy link
Copy Markdown
Collaborator

Summary

  • Add a validated ActionTerm.requires_substep_state_feedback declaration, defaulting to False, and aggregate it in ActionManager.
  • Register the existing pre-step callback only when an active term needs fresh state between physics substeps.
  • Keep Go2WMixedAction on the callback path; invariant actions are applied once and use the existing backend step(ctrl, nsteps=N) path.
  • Cover invariant, mixed-feedback, single-substep, empty-manager, rejection, exception, counter, and close-cleanup behavior.

Linked Work

Validation

  • make check
  • uv run pytest tests/managers/test_core_managers.py tests/envs/test_manager_based_rl_env.py tests/envs/locomotion/go2w/test_go2w_manager_based_flat_cfg.py -q (86 passed)
  • uv run pytest tests/base/test_backend_pre_step_control.py tests/base/test_mjwarp_capabilities.py -q (13 passed, 1 deselected)
  • make test-all (2216 passed, 28 skipped, 275 deselected, 1 xfailed)

Final commit: 27e092bcadb47f0c9379375f21c8dc691b75be95

Benchmark

Fixed command on the same host for both SHAs:

uv run scripts/benchmark/rl/benchmark_offpolicy_collector_active.py --cases flashsac/g1_walk_flat/mujoco --num-envs 8192 --warmup-steps 10 --measure-steps 100 --replay-capacity-steps 64
Metric Before 70836033 After 27e092bc Delta
Collector throughput 68,579 env/s 88,683 env/s +29.3%
Env step 118.420 ms 91.330 ms -22.9%
Backend physics 99.309 ms 73.716 ms -25.8%
Update state 8.458 ms 8.400 ms stable
Reset done 5.172 ms 5.228 ms stable

The timing shift stays in the expected action/dispatch boundary. Raw local JSON was written to /tmp/issue1276-{before,after}.json and is not committed.

Impact

  • Backend impact: MuJoCo invariant-action MBA tasks move from callback dispatch to the existing broadcast trajectory path. Motrix and Drake reuse their existing direct nsteps paths. No backend adapter or SimBackend method changes.
  • Go2WMixedAction remains state-feedback driven and keeps per-substep joint-state reads on MuJoCo, Motrix, and Drake.
  • mjwarp registration/config/support evidence is unchanged. Its configured-only G1 owner may avoid the callback gate, but this PR does not promote support; local runtime smoke could not proceed because the optional mujoco-warp dependency is not installed.
  • Platform impact: Linux validation and benchmark.
  • Training effect: collector throughput only. Policy I/O, action values, rewards, reset/RNG, control decimation, and sim2sim fields are unchanged.

Checklist

  • Added tests at the action-manager and env callback-selection boundaries
  • Linked the driving issue
  • Recorded MuJoCo and non-MuJoCo behavior impact
  • No task/backend allowlist, new execution path, or support claim added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant