Skip to content

Sample DelayBuffer lags on the first step after creation or reset - #1203

Open
Zhuoxi2000 wants to merge 1 commit into
mujocolab:mainfrom
Zhuoxi2000:fix-delay-lag-below-min
Open

Zhuoxi2000 wants to merge 1 commit into
mujocolab:mainfrom
Zhuoxi2000:fix-delay-lag-below-min

Conversation

@Zhuoxi2000

Copy link
Copy Markdown

DelayBuffer starts every row at lag 0, and reset() sets it back to 0. A row's lag only changes on that row's next scheduled refresh. With update_period > 0 and per_env_phase=True (the default), most rows therefore keep lag 0 for up to update_period - 1 steps. With hold_prob > 0, the first refresh can also keep the 0. As a result, even a constant-delay buffer (min_lag=max_lag=2) returns the undelayed frame for many environments at the start of every episode. This contradicts the documented contract: min_lag is "Minimum lag (inclusive)", lags are "sampled uniformly from [min_lag, max_lag]", and the docs say to use min=max for constant delay. It affects both observation terms and actuators that set delay_*. With the example from docs/source/actuators.rst (min 2, max 5, hold 0.3, period 10, 8 envs, seed 0), 7 of 8 envs have lag 0 for the first 7 steps.

This change adds a per-row flag for rows that have not had a lag sampled or set since creation or reset. Those rows always sample on their next compute(), regardless of update phase or hold_prob, and then follow the normal schedule. A staggered env now samples at t=0 and then at its usual phase. A lag set with set_lags() before the first step is still kept, which test_delayed_actuator_set_lags_affects_delay relies on. reset() still reports lag 0 until the next compute(), so the existing reset tests are unchanged. The change adds no random draws and no host sync.

test_lags_stay_within_range_from_first_step covers the staggered update_period case and the hold_prob case, both at creation and after a full reset. Both cases fail on main and pass with the fix. With the fix, tests/test_delay_buffer.py, test_delayed_actuator.py, test_observation_delay.py, test_actuator_builtin_group.py, test_observation_manager.py and test_actuator.py pass, and ruff, ty and pyright are clean.

#1191 also touches _sample_lags, but only the hold draw for per_env=False. This change merges cleanly with it apart from the changelog entry.

AI assistance: this change was drafted with an AI coding assistant (Claude) and verified locally with the tests above.

DelayBuffer started every row at lag 0, and reset() set it back to 0. The lag was only replaced on the row's next scheduled refresh, so with update_period > 0 and per_env_phase=True (the default) most rows kept lag 0 for up to update_period - 1 steps, and with hold_prob > 0 the first refresh could hold the 0. A min_lag=max_lag=2 buffer therefore returned the undelayed frame for many environments at the start of every episode, which breaks the documented [min_lag, max_lag] range for both observation and actuator delays.

Rows that have no lag since creation or reset now always sample on their next compute(), regardless of update phase or hold_prob. A lag set explicitly with set_lags() is still kept. reset() still reports lag 0 until the next compute().

This branch has not been deployed

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

Labels

None yet

1 participant