fix(action): let callers request the action-normalizer forward clamp - #190
Merged
Conversation
#182 made `action_normalization` work for DROID but built the normalizer with the `apply_forward_clamp=False` default, so the dataset does not clamp into [-1, 1]. The cookbooks did clamp while they normalized by hand (NVIDIA/cosmos#314); NVIDIA/cosmos#317 moved them onto the dataset and the clamp silently went away. This threads `apply_forward_clamp` through `BaseActionLeRobotDataset` and `DROIDLeRobotDataset` so the caller names the intent, the way #314 did at its construction site, rather than inheriting it from a default: DROIDMergedLeRobotDataset( root=..., action_normalization="quantile_rot", apply_forward_clamp=True, ) The default stays False, matching `resolve_action_normalization`, so no existing caller changes behavior. On `DROIDLeRobotDataset` the parameter is appended rather than placed next to `action_normalization`: that signature is positional-or-keyword, so inserting mid-list would shift every later argument for positional callers. Measured on the bundled `droid_lerobot_example` (706 stride-1 windows, 112,960 action scalars), clamped vs unclamped: scalars outside q01/q99 1,253 (1.1092%) windows with >=1 outlier 155/706 (21.95%) non-overlapping chunks hit 11/45 (24.4%) max |unclamped| 1.7818 max |difference| 0.78180099 87% of those outliers are on the rot6d channels; the gripper never exceeds the range, its q01/q99 being exactly [0, 1]. The first five chunks -- the ones the cookbooks use -- contain no outliers at all, which is why the existing five-chunk comparison is bit-identical either way and cannot distinguish the two. With `apply_forward_clamp=True` the dataset reproduces #314 exactly: all 706 windows identical, including the 155 where the clamp engages (max|diff| = 0.0000000000). The two normalizers also agree in offset, scale, forward_clamp, forward_clamp_mask and type, so they are the same function on every input, not just the measured ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
xlu451
approved these changes
Aug 13, 2026
lfengad
approved these changes
Aug 13, 2026
pengcuo
added a commit
to NVIDIA/cosmos
that referenced
this pull request
Aug 13, 2026
> **Blocked on NVIDIA/cosmos-framework#190.** Passing `apply_forward_clamp` before that parameter exists raises `TypeError`. Two fixes to the four DROID forward-dynamics notebooks, both regressions from #317. `4 files changed, 8 insertions(+), 2 deletions(-)`. ## 1. Restore two lines dropped from `run_fd_with_cosmos_framework.ipynb` #317 removed the last source line of each of the two cells it edited: - `return records` — end of `create_record_from_dataset` - `print(droid_fd_input_path.read_text())` — end of the DROID plan cell Without the return, `create_record_from_dataset` yields `None` and both call sites fail: ```python droid_records = create_record_from_dataset(droid_dataset, num_chunks, chunk_length) droid_fd_input_path.write_text("".join(json.dumps(r) + "\n" for r in droid_records)) # TypeError: 'NoneType' object is not iterable ``` The **UMI** cell shares the same helper and breaks identically, though it has nothing to do with DROID action normalization. This notebook cannot currently be run end to end on `main`. ### Why only this notebook The two lost lines are exactly the two whose `source` entry had no trailing newline. The cells edited in the other three notebooks all ended with one and came through intact: | notebook | cell | last line before | dropped | |---|---|---|---| | cosmos_framework | 12 | ` return records` | **yes** | | cosmos_framework | 32 | `print(droid_fd_input_path.read_text())` | **yes** | | diffusers | 41 | `display(droid_first_frame)\n` | no | | sglang | 15 | `print(robotics_fd_input_path.read_text())\n` | no | | vllm_omni | 16 | `print(robotics_fd_input_path.read_text())\n` | no | That is the signature of a `"".join(source).split("\n")[:-1]` round-trip: when the last entry ends in `\n` the trailing element is an empty string and slicing it off is harmless, otherwise it discards a real line. Worth knowing for whatever tooling edits these notebooks — it will do this again. ## 2. Ask the dataset to forward-clamp DROID actions #314 built the normalizer by hand with `apply_forward_clamp=True`. #317 moved that onto the dataset, which builds it with the `False` default, so the notebooks stopped clamping into `[-1, 1]`. Passing the flag matches #314 again. Measured on the bundled `droid_lerobot_example` — 706 stride-1 windows, 112,960 action scalars — clamped vs unclamped: | | | |---|---| | scalars outside q01/q99 | 1,253 (1.1092%) | | windows with ≥1 outlier | 155/706 (21.95%) | | non-overlapping chunks hit | 11/45 (24.4%) | | max \|unclamped\| | 1.7818 | | max \|difference\| | 0.78180099 | 87% of those outliers are on the rot6d channels; the gripper never exceeds the range, its q01/q99 being exactly `[0, 1]`. The first five chunks — the ones these notebooks use — contain **no** outliers at all. That is why a five-chunk comparison comes out bit-identical either way and cannot distinguish clamped from unclamped, which is what the existing verification measured. ## Verification With the flag, the dataset reproduces #314 exactly on the bundled fixture: all **706/706** windows identical, including the **155** where the clamp engages, `max|diff| = 0.0000000000`. The two normalizers also agree in `offset`, `scale`, `forward_clamp`, `forward_clamp_mask` and type, so they are the same function on every input. Structural checks on the notebooks: cell counts unchanged, every cell's last source line unchanged, all code cells parse, `create_record_from_dataset` returns `records` again. All four notebook dataset paths were then constructed with their own arguments and confirmed to build a clamping normalizer: | notebook | construction | `forward_clamp` | |---|---|---| | cosmos_framework | `DROIDMergedLeRobotDataset(root, chunk_length, ...)` | `(-1.0, 1.0)` | | sglang | `DROIDMergedLeRobotDataset(root, chunk_length, ...)` | `(-1.0, 1.0)` | | vllm_omni | `DROIDMergedLeRobotDataset(root, ...)` — no `chunk_length` | `(-1.0, 1.0)` | | diffusers | `DROIDLeRobotDataset(root, chunk_length, use_success_only=True, ...)` | `(-1.0, 1.0)` | `run_fd_with_diffusers.ipynb` is the one that differs: it uses `DROIDLeRobotDataset` rather than the merged subclass, and that class picks its feature layout from the directory basename, so it rejects `droid_lerobot_example` outright with `ValueError: Unknown version`. It was exercised by reproducing the release layout the notebook itself builds — a `droid_plus_lerobot_640x360_20260412/success` symlink to the bundled sample — which it accepts (706 windows, same episode). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <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
3 participants
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.
Summary
#182 made
action_normalizationwork for DROID, but built the normalizer with theapply_forward_clamp=Falsedefault, so the dataset does not clamp into[-1, 1].The cookbooks did clamp while they normalized by hand (NVIDIA/cosmos#314). NVIDIA/cosmos#317 moved them onto the dataset, and the clamp went away with the hand-written code.
This threads
apply_forward_clampthroughBaseActionLeRobotDatasetandDROIDLeRobotDatasetso the caller names the intent, the way #314 did at its construction site:No default behavior change
The default stays
False, matchingresolve_action_normalization, so every existing caller keeps its current behavior:forward_clampNone(identical tomain)apply_forward_clamp=True(-1.0, 1.0)action_normalization=NoneOn
DROIDLeRobotDatasetthe parameter is appended rather than placed next toaction_normalization: that signature is positional-or-keyword, so inserting mid-list would shifttolerance_s,viewpoint,use_success_onlyand eight more for any positional caller.BaseActionLeRobotDataset.__init__is keyword-only, so the placement there is free.Why it matters
Measured on the bundled
droid_lerobot_example— 706 stride-1 windows, 112,960 action scalars — clamped vs unclamped:87% of those outliers are on the rot6d channels, led by R21 (496) and R20 (336). The gripper never exceeds the range — its q01/q99 is exactly
[0, 1]. Nothing raises on this path: the values stay plausible floats through the transform, the serving JSON and the model.The rot6d diagonal channels are the ones with headroom to get much worse. R00/R11 have a q01/q99 half-width of ~7.7e-4, so they saturate at roughly 3.2° of per-frame rotation; at 15 fps, 10°/frame normalizes to about -19 unclamped. This fixture never rotates that fast — its worst case is 1.78 — so that figure is arithmetic from the stats, not a measurement.
Verification
With
apply_forward_clamp=Truethe dataset reproduces #314 exactly on the bundled fixture:max|diff| = 0.0000000000offset,scale,forward_clamp,forward_clamp_maskand type, so they are the same function on every input, not just the measured onesThe first five chunks — the ones the cookbooks use — contain no outliers at all, so a five-chunk comparison is bit-identical either way and cannot distinguish clamped from unclamped.
Existing action tests (43) pass.
Follow-up
NVIDIA/cosmos will pass the flag from the four DROID forward-dynamics notebooks once this lands; that PR is blocked on this one.
🤖 Generated with Claude Code