fix(tasks): honor reset(states=) contract in eight Tier-1 task overrides - #809
Merged
Conversation
BaseTaskEnv.reset(states=None, env_ids=None, seed=None) is the contract every task must be substitutable for, but eight Tier-1 overrides dropped `states`: the two legged-robot bases (humanoid, beyondmimic) reimplemented reset without it, and the six SimplerEnv tasks took `options` in its place. The IL and VLA evaluation runners pass `states` unconditionally — default_runner calls env.reset(states=init_states[...]) to replay a demo's initial states, and the OpenVLA, SmolVLA and pi0 eval loops do the same. Because `--task` is a free-form string, nothing stopped a user from pairing a VLA with a SimplerEnv task — the canonical pairing for that benchmark — and doing so died immediately with `TypeError: reset() got an unexpected keyword argument 'states'`. The same TypeError hit any IL evaluation on the humanoid or beyondmimic bases. All eight now accept states/env_ids/seed in the base's order. The legged-robot bases apply caller-supplied states in place of their scripted initial states. The SimplerEnv tasks keep `options` — it carries the episode spec (object/robot init options, episode id) that the gym adapter feeds them, so dropping it would have silently discarded per-episode setup — but it moves after the contract's three parameters and keeps its default. External states are applied on top of the scripted layout, and the settle-derived quantities the success checkers key off (object height, object poses, bbox extents) are re-derived from the applied states rather than left describing a layout that was overwritten. The guardrail that should have caught this (tests/test_task_reset_seed_contract.py) only looked for a `seed` parameter, so all eight sailed through it — that is why they rotted. It now checks full substitutability: every contract parameter is accepted, positional order matches the base (a swapped positional binds the wrong value, which corrupts rather than raises), and an override may not add a required parameter of its own. Extra parameters like SimplerEnv's `options` remain allowed after the contract's three, with defaults. The ratchet is zero-tolerance, so this class of drift cannot recur.
geng-haoran
force-pushed
the
fix/task-reset-states-contract
branch
from
September 3, 2026 19:23
7ee6127 to
09720eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BaseTaskEnv.reset(states=None, env_ids=None, seed=None) is the contract every task must be
substitutable for, but eight Tier-1 overrides dropped
states: the two legged-robot bases(humanoid, beyondmimic) reimplemented reset without it, and the six SimplerEnv tasks took
optionsin its place.The IL and VLA evaluation runners pass
statesunconditionally — default_runner callsenv.reset(states=init_states[...]) to replay a demo's initial states, and the OpenVLA, SmolVLA
and pi0 eval loops do the same. Because
--taskis a free-form string, nothing stopped a userfrom pairing a VLA with a SimplerEnv task — the canonical pairing for that benchmark — and doing
so died immediately with
TypeError: reset() got an unexpected keyword argument 'states'. Thesame TypeError hit any IL evaluation on the humanoid or beyondmimic bases.
All eight now accept states/env_ids/seed in the base's order. The legged-robot bases apply
caller-supplied states in place of their scripted initial states. The SimplerEnv tasks keep
options— it carries the episode spec (object/robot init options, episode id) that the gymadapter feeds them, so dropping it would have silently discarded per-episode setup — but it
moves after the contract's three parameters and keeps its default. External states are applied
on top of the scripted layout, and the settle-derived quantities the success checkers key off
(object height, object poses, bbox extents) are re-derived from the applied states rather than
left describing a layout that was overwritten.
The guardrail that should have caught this (tests/test_task_reset_seed_contract.py) only looked
for a
seedparameter, so all eight sailed through it — that is why they rotted. It now checksfull substitutability: every contract parameter is accepted, positional order matches the base
(a swapped positional binds the wrong value, which corrupts rather than raises), and an override
may not add a required parameter of its own. Extra parameters like SimplerEnv's
optionsremainallowed after the contract's three, with defaults. The ratchet is zero-tolerance, so this class
of drift cannot recur.
Review: independently reviewed against current main (verdict MERGE AS-IS); rebased, re-verified: 14 passed in 6.72s
🤖 Generated with Claude Code
https://claude.ai/code/session_017i6VtKoovBNed815mWFqxw