fix(data): clamp history delta_indices at episode start instead of wrapping to the episode tail - #772
Open
elbourne12345 wants to merge 1 commit into
Conversation
…apping extract_step_data passed `step_index + delta_index` straight to DataFrame.iloc when allow_padding=False. pandas counts negative positions from the END of the frame, so a history offset such as the shipped DROID video delta_indices=[-15, 0] returned frames from the last 15 rows of the episode for every step_index < 15 - silently, on the default training path (DataConfig.allow_padding is False and launch_finetune.py never changes it). Both in-tree deployment paths pad history by repeating the first observation (MultiStepWrapper.reset, examples/DROID/main_gr00t.py), so this was also a train/inference mismatch. N1.5 clamped front indices to frame 0 via retrieve_data_and_pad(..., "first_last"); that behaviour was lost when 4e62473 replaced gr00t/data/dataset.py. With allow_padding=False, negative observation indices are now clamped to frame 0 and any other out-of-range index raises IndexError instead of wrapping. allow_padding=True is unchanged, as is the sharder's step schedule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Fixes #771
Changes proposed in this pull request:
extract_step_datano longer passes negative positions toDataFrame.ilocwhenallow_padding=False. Observation history that reaches before the first frame is clamped to frame 0; any other out-of-range index raisesIndexErrornaming the modality and index.allow_padding=Trueis unchanged, as is the sharder's step schedule.allow_padding=Falsesemantics onDataConfig.allow_paddingand in theShardedSingleStepDatasetdocstring.getting_started/data_config.mdno longer states that no N1.7 embodiment config uses negative indices (embodiment_configs.py:30ships[-15, 0]), and now describes the start-of-episode padding.tests/gr00t/data/test_extract_step_data_history.py(10 tests, CPU-only).The bug
indices_to_load = [step_index + delta ...]went straight toDataFrame.iloc. pandas treats negative positions as offsets from the end of the frame, so history offsets at the start of an episode returned frames from its tail.DataConfig.allow_paddingdefaults toFalseandlaunch_finetune.pynever changes it, so this is the default training path. The in-repooxe_droid_relative_eef_relative_jointconfig hasvideo: delta_indices=[-15, 0]andexamples/DROID/README.mddocuments finetuning with that tag: for every DROID episode, steps 0–14 were trained with a "previous frame" taken from the last 15 frames of the episode. The sharder schedules all of them —get_effective_episode_lengthonly subtracts the action horizon.Measured on
demo_data/droid_sampleepisode 1 (266 frames) at 51d4c89, mirroring the video offsets onto the state stream so the repro needs notorchcodec(the index list is computed once per modality and applied identically to all of them):Both in-tree deployment paths pad history by repeating the first observation (
MultiStepWrapper.reset, the DROID client'sframe_buffer), so this was also a train/inference mismatch. Offline eval hits the same wrong frame at step 0 (open_loop_eval.py,standalone_inference_script.py,export_onnx_n1d7.pyuse the default;benchmark_inference.pyand the two inference notebooks passallow_padding=Falseexplicitly).This is a regression: N1.5's
gr00t/data/dataset.pypadded front indices with frame 0 viaretrieve_data_and_pad(..., "first_last"); 4e62473 (#456) replaced it with the unguardedilocpath.Behaviour changes, stated explicitly
With
allow_padding=False:|min(delta)|: previously raised (pandas out-of-bounds), now clamp to frame 0 like every other early step. Unreachable from the sharder (effective length would be 0), reachable from the eval scripts.gr00t/eval/_horizon_contract.pyalready rejects them at inference.IndexErrorin pandas; only the message changes.benchmark_inference.py'sexcept Exceptiontrajectory loop stops at the same step as before.Alternatives
Defaulting
DataConfig.allow_paddingtoTrueis a one-line change that gives identical results on every sharder-scheduled step. I did not take it because it would also silently clamp action windows past the episode end, which the eval scripts currently rely on failing loudly. Happy to switch if you prefer the one-liner, or to drop the doc commit if you would rather change the shippedoxe_droid_relative_eef_relative_jointconfig tovideo: [0](matching the releasednvidia/GR00T-N1.7-DROIDcheckpoint) — though that would leave custom history configs affected.Interaction with #743
LeRobotEpisodeLoaderFaster._get_video_indices_from_stepspasses negative frame indices through whenallow_padding=False, andtest_no_padding_keeps_negative_indicesasserts that. If #743 lands, that loader needs the same clamp — otherwise the clamped step-0 history would reference a frame it never decoded (None). Happy to rebase in whichever order suits you.Testing
tests/gr00t/data/test_extract_step_data_history.py:main(returns the episode-tail rows);ShardedSingleStepDataset.get_shardwith a mocked episode loader and the defaultallow_padding: the early steps are still scheduled and now get frame 0 as history;allow_padding=Truestill clamps both ends.python -m pytest tests/gr00t/data/test_extract_step_data_history.py tests/gr00t/data/test_sharded_datasets.py -q→ 30 passed.demo_data/droid_samplereproduction from Negative delta_indices (history frames) silently read from the end of the episode during training #771 after the fix: all four steps return row 0 as the history frame; the sharder schedule is unchanged.ruff checkandruff format --checkclean on the touched files.I could not run the GPU suite or measure the effect on a finetuned policy — that needs a DROID training run.
Before submitting
section of the
CONTRIBUTINGdocs. (FYI:CONTRIBUTING.mdcurrently has no "Making a pull request" section, so that template link 404s to the top of the file.)Generated with assistance from Claude Code