Skip to content

Share render-state publication across active visual consumers - #667

Merged
acrlw merged 4 commits into
DexForce:mainfrom
acrlw:fix/locomotion-render-sync
Sep 23, 2026
Merged

acrlw merged 4 commits into
DexForce:mainfrom
acrlw:fix/locomotion-render-sync

Conversation

@acrlw

@acrlw acrlw commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Headless state-only RL was publishing the full Newton physics scene to the renderer even without visual consumers. This PR removes that work and gives windows, cameras, recording and Viser a shared publication phase. With NewtonPhysicsCfg(sync_to_renderer=False), an open native window now receives current state continuously; combined consumers publish once per frame instead of two or three times.

SimulationManager.render_frame() is a read-only consumption context. Gym defers rendering of the last physics substep until interval events finish, then obtains camera observations inside the context; due recording and Viser captures consume the same state on exit. Reset finishes its state writes before opening a new frame. Viser checks whether a frame is due before requesting publication. Manager-owned Newton stepping suppresses DexSim's automatic publication and honors an explicit sync_to_renderer=True request within the shared phase. Raw DexSim World.update() keeps its existing policy.

Publication is never cached across frames or independent reads. Direct edits inside a frame require an explicit sync_render_state() call, which remains unconditional. Independent callers can combine update(render_final_step=False) with render_frame() to share camera/recording/Viser publication. Render-thread recording consumes already-published state and never invokes the blocking bridge from that thread. A failed frame clears its publication flag; failed observations retain pending recording time.

This addresses both review findings: the missing continuous window publication and repeated publication by combined consumers. Dependencies, physics/PPO parameters and DexSim source are unchanged. API usage, configuration semantics and agent context are updated. No associated issue.

Validation of implementation 2033b804 (PR head 4d9a9eec)

The follow-up commits 02afa235 and 4d9a9eec complete the profiler and Warp-log test fixtures for backend-owned stepping. Runtime implementation is unchanged. The final configuration/profiler validation passes all 37 tests.

  • 446 local checks pass: 305 synchronization/visualization/Gym/context checks, 133 additional Gym checks and 8 API-checker tests. The two reviewer regressions fail against the previous PR head 83e396a7 and pass with this revision. Coverage includes all three Newton synchronization policies, window/headless modes, consumer combinations, interval-event edits, paused edits, explicit publication and failed-frame retry.
  • Real RTX 5090 D v2 / Newton MJWarp CUDA / FastRT checks pass for headless stepping, a native window, and camera + simulation-time recording + Viser. Window and combined modes publish once at each of six steps; GPU render transforms match physics. Paused direct edits update both render transforms and captured images. The combined run saves six video frames.
  • G1, 4,096 environments, 24 steps, seed 42, four CPU threads: both physics backends complete five PPO updates on both the previous and revised source. All ten revised rollouts make zero explicit render publications, including rollouts with partial resets. All four checkpoints reload with finite policy and optimizer tensors. Newton CUDA/MJWarp execution and the default GPU path are checked explicitly.
  • The revised Newton run records 2.483 s for three rollouts and 0.254 s for three PPO updates; the revised default GPU run records 8.324 s and 0.266 s. These are functional short-run records, not an uncontended A/B benchmark: external GPU jobs appeared during the previous Newton run and the revised default run. No new speedup ratio is claimed.
  • Black 26.3.1: all 1,119 files pass the repository-wide read-only check. API coverage: 2,186/2,186 exports. The full Sphinx build passes on implementation commit 2033b804 in CI. The current-head CI run passes tests, Test gate, build, lint and documentation checks.

Versions remain pinned to DexSim c8e8bc11, Newton 1.6.0, MuJoCo/MJWarp 3.12.0, Warp 1.17.0 and Torch 2.10.0+cu128. Source paths and 145 task/configuration files are verified. Revised runtime checks used the local source changes subsequently committed as 2033b804; original provenance retains the pre-commit head and diff.

Existing default-backend boundary

A separate FastRT/default-GPU probe reproduces a paused rigid-body pose discrepancy on both 83e396a7 and this revision: the physics readback reflects a direct pose write, while the render transform and image retain the prior pose. A subsequent physics step in that probe does not resolve the discrepancy. Ordinary stepping without that direct edit passes the camera/recording/Viser check. This PR fixes publication scheduling; it does not add a missing native default-backend pose-publication implementation. Paused direct edits on that backend are not marked accepted.

Earlier performance and server evidence

The original fix at 83e396a7 already completed the isolated Pro5000 GPU1 batch: Go2, Go1, G1 and H1_2 × both backends, eight 100-update training/evaluation/FastRT-video runs and six independent performance repeats. G1/Newton three-rollout median fell from 102.743 s to 3.402 s (30.2×), with zero explicit publications across 800 training rollouts. That evidence supports removal of the original headless bottleneck; it is not server validation of 2033b804. The revised implementation at 02afa235 completed Pro5000 GPU1 revalidation: all eight combinations of four robots and two backends passed smoke checks, 100-update training, FastRT evaluation and video generation. All 800 rollouts recorded zero explicit render publications; 32 checkpoints passed reload/finite-value checks and all eight videos decoded completely. The planned six additional performance repeats did not run because the performance preflight still pinned the earlier source revision. Current head 4d9a9eec has the same runtime code as this completed batch. Policy convergence and manual keyboard play remain outside these short-run acceptance results.

Type of change

  • Bug fix

Screenshots

Local reports retain horizontal publication-count and training-resource charts, raw measurements, pose images and FastRT recordings. The publication matrix changes from 0 to 1 for an open window with automatic synchronization disabled, 2 to 1 for recording + camera or recording + Viser, and 3 to 1 for all three. State-only headless frames remain at zero.

Checklist

  • Changed Python files are formatted; the repository-wide Black result is recorded.
  • API documentation, configuration documentation and agent context are updated.
  • Regression tests demonstrate the reviewer findings and their fixes.
  • Dependencies are reviewed; no updates are required.

@acrlw acrlw added bug Something isn't working rendering Things related to rendering (eg, performace, efficiency, bug) rl Features related to reinforcement learning labels Sep 22, 2026
@acrlw
acrlw marked this pull request as ready for review September 22, 2026 04:28
@greptile-apps

greptile-apps Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no new actionable issue was introduced since the previous review.

Summary

The PR centralizes render-state publication in a shared, read-only frame context so windows, cameras, recording, and Viser can consume one publication while state-only headless stepping avoids unnecessary renderer synchronization.

  • Adds SimulationManager.render_frame() and defers Gym’s final-substep rendering until interval-event writes are complete.
  • Moves backend-owned stepping behind physics hooks, suppressing automatic Newton publication during manager-owned updates.
  • Makes Viser request publication only when a capture is due and keeps render-thread recording from invoking the blocking publication bridge.
  • Adds focused coverage for consumer combinations, failed-frame retries, recording cadence, visualization deadlines, and Newton synchronization policies.
  • The changes since the previous review only complete test fixture setup; they do not alter runtime behavior.
Diagram
sequenceDiagram
    participant Env as Gym / Caller
    participant Sim as SimulationManager
    participant Physics as Physics Backend
    participant Render as Render State
    participant Consumers as Window / Camera / Recording / Viser

    Env->>Sim: "update(..., render_final_step=False)"
    loop Physics substeps
        Sim->>Physics: _step(dt)
        Physics->>Physics: Advance world without automatic publication
    end
    Env->>Env: Apply interval events or reset writes
    Env->>Sim: enter render_frame()
    Env->>Sim: Render camera observations
    Sim->>Render: Publish state on first due consumer
    Render-->>Consumers: Shared current state
    Sim->>Consumers: Due recording and Viser capture
    Env->>Sim: exit render_frame()
    Sim->>Sim: Clear frame publication flag
Loading

Reviews (4) · Last reviewed commit: "test(rendering): complete backend step f..."

@acrlw
acrlw requested a review from yuecideng September 22, 2026 05:24
Comment thread embodichain/lab/sim/sim_manager.py
Comment thread embodichain/lab/sim/sim_manager.py Outdated
@acrlw acrlw changed the title Avoid render-state publication without visual consumers Share render-state publication across active visual consumers Sep 22, 2026
@acrlw

acrlw commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Both review findings are addressed in 2033b804; current head 02afa235 completes the profiler test stubs identified by CI. The changes are in EmbodiChain's publication scheduling. DexSim source and physics/PPO parameters are unchanged.

All 446 local checks pass. Real Newton/MJWarp CUDA + FastRT validation covers the native window, offscreen cameras, recording, Viser and paused direct pose edits. With G1 and 4,096 environments, both backends complete five PPO updates each. All ten revised rollouts make zero explicit render publications; checkpoint reload and finite policy/optimizer tensor checks pass.

One confirmed boundary remains: on the default GPU backend, a direct rigid-body pose write while paused updates the physics readback, but FastRT render transforms and images retain the previous pose. This reproduces on both the previous PR head and the revision and remains unfixed. Newton window synchronization and combined offscreen consumption pass the current checks; paused direct pose edits on the default backend remain an unaccepted case.

The full documentation build passed on implementation commit 2033b804; the complete CI result for the current PR head still needs confirmation. PR head 02afa235 is now deployed in a separate Pro5000 GPU1 worktree. All eight environment checks passed, and the first Go2/Newton 100-update training run has completed. The remaining serial revalidation batch is continuing. External GPU jobs appeared during local timing runs, so no new speedup ratio is claimed.

Comment thread embodichain/lab/sim/sim_manager.py
@acrlw
acrlw merged commit 53330ae into DexForce:main Sep 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rendering Things related to rendering (eg, performace, efficiency, bug) rl Features related to reinforcement learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants