Skip to content

feat(sim): auto-disable mujoco shadows when rendering is slow - #4232

Open
spomichter wants to merge 3 commits into
mainfrom
sim-auto-disable-shadows
Open

spomichter wants to merge 3 commits into
mainfrom
sim-auto-disable-shadows

Conversation

@spomichter

Copy link
Copy Markdown
Contributor

@bogwi Dan — heads up, this is a slop PR (Claude-authored from a debugging session on my Mac) — shared in case it's helpful. Take what's useful or close it.

Problem

dimos --simulation run unitree-go2 runs at ~0.4x realtime on Apple Silicon. Profiling showed physics is fine (0.8 ms/step, 6.3x realtime headroom with the office scene) — the cost is shadow-mapping the ~1220-mesh scene in the offscreen camera renders: 28.6 ms with shadows vs ~7 ms without per 640x360 frame, out of a 50 ms video frame budget.

Changes

  • mujoco_shadows: Literal["auto", "on", "off"] = "auto" in GlobalConfig. auto benchmarks one shadowed offscreen render at sim startup (~150 ms) and zeroes model.vis.quality.shadowsize when it exceeds 30% of the video frame budget. This must happen before launch_passive/Renderer create their GL contexts — the shadow framebuffer is allocated at context creation, so toggling mjRND_SHADOW scene flags at runtime leaves the interactive viewer shadow-rendering the whole scene regardless (tried that first; didn't help).
  • Skip CoreMLExecutionProvider for the locomotion policy ONNX session: CoreML partitions the 14-node graph (13/14 nodes) and pays a per-call copy overhead making it ~20x slower than plain CPU inference (0.335 ms vs 0.014 ms).

Machines where the shadowed render is cheap (discrete GPUs measure ~2-5 ms) keep shadows; nothing changes for them.

Measured (M-series Mac, go2 + office1 scene)

before after
worst-case loop iteration 92 ms 28 ms
realtime factor 0.38x 1.25x

Verified end-to-end: startup logs Shadowed render took 28.6 ms of the 50 ms frame budget; disabling shadows and the sim runs at realtime.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
6086 1 6085 103
View the full list of 1 ❄️ flaky test(s)
dimos.e2e_tests.test_voice_browser::test_hold_to_talk_ships_a_decodable_recording[firefox]

Flake rate in main: 68.75% (Passed 5 times, Failed 11 times)

Stack Traces | 22.7s run time
voice_bridge = ('http://127.0.0.1:62113/', [])
fake_mic_page = <Page url='http://127.0.0.1:62113/'>

    def test_hold_to_talk_ships_a_decodable_recording(
        voice_bridge: tuple[str, list[AudioChunk]], fake_mic_page: Page
    ) -> None:
        url, chunks = voice_bridge
        chunks.clear()  # the module fixture is shared across both engines
        fake_mic_page.goto(url)
        mic = fake_mic_page.get_by_test_id("chat-audio_in-mic")
        # Enabled == transport connected; the manifest already placed the panel.
        expect(mic).to_be_enabled(timeout=120_000)
        expect(mic).to_have_attribute("data-state", "idle")
    
        mic.hover()
        fake_mic_page.mouse.down()
>       expect(mic).to_have_attribute("data-state", "recording", timeout=15_000)
E       AssertionError: Locator expected to have attribute 'recording'
E       Actual value: arming 
E       Call log:
E         - Expect "to_have_attribute" with timeout 15000ms
E         - waiting for get_by_test_id("chat-audio_in-mic")
E       
E       Aria snapshot:
E       - button "hold to talk": talk

chunks     = []
fake_mic_page = <Page url='http://127.0.0.1:62113/'>
mic        = <Locator frame=<Frame name= url='http://127.0.0.1:62113/'> selector='internal:testid=[data-testid="chat-audio_in-mic"s]'>
url        = 'http://127.0.0.1:62113/'
voice_bridge = ('http://127.0.0.1:62113/', [])

dimos/e2e_tests/test_voice_browser.py:104: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@spomichter
spomichter force-pushed the sim-auto-disable-shadows branch from 337eea0 to 6d25d29 Compare September 19, 2026 09:01

@bogwi bogwi Sep 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
frame_sim_time = model.opt.timestep * config.mujoco_steps_per_frame
time_until_next_step = frame_sim_time - (time.time() - step_start)

Drop shadows on a slow machine and drop core ml are fine fixes. Yet they surfaced another latent bug int the render loop.

Each iteration of the main loop steps physics mujoco_steps_per_frame times: 7 x 0.005 s - so 35 ms of sim time. The sleep at the end pads the iteration to model.opt.timestep only, 5 ms of wall time - so the loop was never paced to realtime. Its speed was whatever the viewer sync and the offscreen renders happened to cost per machine.

With shadows on, on Apple macs, render was slow that one iteration took about 35 ms and the sim looked close to realtime. With shadows off the iteration takes about half of that, the same 35 ms of sim time is consumed per iteration and the go2 runs at about 2x.

On my m4 Pro this is go2 bolt running the office. On your Air - the iteration is slower, so it probably looked fine. Linux with a discrete GPU most likely ran above realtime the whole time and nobody had a before and after to compare against.

Add these two lines and go2 will 1) move with correct speed and 2) movement + Shift correctly paces up too.

@bogwi bogwi Sep 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might go as a separate PR as well, yes, though the changes surfaced the bug, so let's not leave it for later

spomichter and others added 2 commits September 20, 2026 02:32
Shadow-mapping the office scene costs ~4x per offscreen render on
integrated GPUs (28.6ms vs 7ms per 640x360 frame on Apple Silicon),
pinning the whole sim at ~0.4x realtime. Benchmark one shadowed render
at startup and zero shadowsize before the viewer/renderer GL contexts
are created when it exceeds 30% of the video frame budget.

Also skip CoreMLExecutionProvider for the locomotion policy: it
partitions the 14-node graph and is ~20x slower than plain CPU
inference (0.335ms vs 0.014ms per call).
@bogwi
bogwi force-pushed the sim-auto-disable-shadows branch from 6d25d29 to a25ae8f Compare September 19, 2026 17:33
@bogwi
bogwi marked this pull request as ready for review September 19, 2026 17:33
@bogwi bogwi added the backport:skip Skip creating a backport to any release branches label Sep 19, 2026
@greptile-apps

greptile-apps Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The remaining findings are non-blocking documentation and regression-coverage improvements; they do not make the implementation unsafe to merge.

Findings

  1. P2 Document shadow setting
  2. P2 Test shadow selection

Summary

This change adds adaptive MuJoCo shadow control, simulation pacing updates, and CoreML avoidance for locomotion policies. Two non-blocking follow-ups remain: document the new public shadow override and add focused regression tests for its selection behavior.

Reviews (1) · Last reviewed commit: "fix control simulation speed"

Comment thread dimos/core/global_config.py
Comment thread dimos/simulation/mujoco/mujoco_process.py
@dimensionalOS dimensionalOS deleted a comment from greptile-apps Bot Sep 19, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 19, 2026
Comment on lines +267 to +268
frame_sim_time = model.opt.timestep * config.mujoco_steps_per_frame
time_until_next_step = frame_sim_time - (time.time() - step_start)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes it so the speed is no longer faster than real time, making the robot move quite slowly.

@paul-nechifor paul-nechifor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The speed of the robot is too low now.

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

Labels

backport:skip Skip creating a backport to any release branches PlzReview ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants