Repository navigation
[Bugfix][Model] GR00T-N1.7: honor processor_config image pipeline (use_albumentations / crop_fraction / letter_box_transform) to match Isaac-GR00T - #7083
Conversation
…match Isaac-GR00T Gr00tN1d7Processor dropped the checkpoint's `use_albumentations` and `letter_box_transform` into `**kwargs` and always ran a torchvision recipe (letterbox -> Resize(256, bilinear) -> CenterCrop(230) -> Resize(256)). Both public N1.7 checkpoints ship `use_albumentations: true`, `crop_fraction: 0.95`, `shortest_image_edge: 256`, `image_crop_size: [230, 230]`, `letter_box_transform: false`, which in Isaac-GR00T selects SmallestMaxSize(256, INTER_AREA) -> FractionalCenterCrop(0.95) -> SmallestMaxSize(256, INTER_AREA): a 243 px crop rather than 230 (0.95 vs 0.898 of the frame), area resampling rather than bilinear, and no letterbox. The model was therefore served out-of-distribution pixels. Measured on GB200 against nvidia/GR00T-N1.7-DROID with Isaac-GR00T 51d4c89 as the reference, on one non-zero frame per view and with the reference's own initial denoising noise injected: `pixel_values` differed by 1.945, `backbone_features` by 16.78 and the decoded actions by 0.107-0.489, while every other model input was already bitwise equal. With this change `pixel_values` is bitwise equal to the reference. Select the recipe the way Isaac-GR00T's processor does. `use_albumentations` now builds AlbumentationsEvalImageTransform, a cv2/numpy transcription of Isaac-GR00T's eval branch and albumentations 1.4.18 (SmallestMaxSize rounding, FractionalCenterCrop truncation, LetterBoxPad) including the crop_fraction and shortest_image_edge fallbacks; no albumentations dependency is added and cv2 is already available through vLLM. The torchvision recipe now gates its letterbox on `letter_box_transform`, as Isaac-GR00T does. Both flags are stored and persisted by save_pretrained so save/load round-trips. Checkpoints without the keys keep the previous torchvision behavior, the defaults matching Isaac-GR00T. The per-frame contract at the single call site is unchanged. The SPDX copyright line change is applied by the check-spdx-header hook, which rewrites the legacy vLLM line on any file it touches. Signed-off-by: liangmengh <liangmengh@nvidia.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/diffusion/diffusion_model_integration.md. Module owners: @fhfuih @Bounty-hunter @wtomin @RuixiangMa @liangmenghuang, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
|
Self-review. What changed: What I checked:
What I did not check: whether this changes any rollout benchmark outcome. The |
|
@tzhouam PTAL |
timzsu
left a comment
There was a problem hiding this comment.
This PR adds support for two transform options that my initial porting missed. The change is easy to read through and reproduction plus end-to-end precision numbers are also provided. LGTM overall.
I have left a minor comment on the docstring which seems to reference a non-existent file. PTAL when you have time.
…e regenerated The test module docstring asked readers to keep the frame generators byte-identical to "the regeneration script attached to the PR". That script is not a file in this repository, and the wording reads like an in-tree path, so a reviewer following it finds nothing; the block has been dropped from the PR description as well. State the procedure in the docstring instead: regenerating a digest needs Isaac-GR00T, whose transformers pin conflicts with this repo's, so it cannot run here; in an Isaac-GR00T environment push each synthetic_frames() frame through that eval transform with letter_box_transform False and True and hash the output with digest(). Docstring only: GOLDEN, the frame generators and every test are unchanged. Signed-off-by: liangmengh <liangmengh@nvidia.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e_albumentations / crop_fraction / letter_box_transform) to match Isaac-GR00T (vllm-project#7083) Signed-off-by: liangmengh <liangmengh@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: ZhengWG <zwg0606@gmail.com>
…e_albumentations / crop_fraction / letter_box_transform) to match Isaac-GR00T (vllm-project#7083) Signed-off-by: liangmengh <liangmengh@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…e_albumentations / crop_fraction / letter_box_transform) to match Isaac-GR00T (vllm-project#7083) Signed-off-by: liangmengh <liangmengh@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: wenjie.yan <wenjyan@outlook.com>
…e_albumentations / crop_fraction / letter_box_transform) to match Isaac-GR00T (vllm-project#7083) Signed-off-by: liangmengh <liangmengh@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Purpose
Fixes #7081.
What broke
Gr00tN1d7Processorignores the image-pipeline keys of the checkpoint'sprocessor_config.json:use_albumentationsandletter_box_transformare swallowed by**kwargs, and the eval transform is alwaysLetterBox -> Resize(256, bilinear) -> CenterCrop(230) -> Resize(256). Both public N1.7 checkpoints (nvidia/GR00T-N1.7-3B,nvidia/GR00T-N1.7-DROID) shipuse_albumentations: true, crop_fraction: 0.95, shortest_image_edge: 256, image_crop_size: [230, 230], letter_box_transform: false, which in Isaac-GR00T meansSmallestMaxSize(256, INTER_AREA) -> FractionalCenterCrop(0.95) -> SmallestMaxSize(256, INTER_AREA): a 243 px crop (not 230; 230/256 = 0.898 vs 0.95), area resampling (not bilinear), no letterbox. The model gets out-of-distribution pixels.Measured on GB200 with the DROID checkpoint against Isaac-GR00T 51d4c89, on one non-zero frame per view and with the reference's own initial denoising noise injected so the ODE starts from the same point:
pixel_valuesmax abs diff 1.945,backbone_features16.78, decoded actions 0.107-0.489, while every other model input (input_ids,attention_mask,image_grid_thw,state,embodiment_id) is already bitwise equal — which places the divergence in the image pipeline. Full numbers, including the arm of the same experiment run on this branch, are under Test Result.Repro (main, no Isaac-GR00T needed; cv2 ships with vLLM)
_build_eval_image_transformkeeps its positional signature, so the second call behaves the same on main and on this branch; the flags are keyword-only and come fromprocessor_config.json.Root cause
The port re-implemented Isaac-GR00T's
build_image_transformations(torchvision recipe) with the letterbox hard-wired on, and never portedbuild_image_transformations_albumentations, the recipe selected byuse_albumentations=True. The processor__init__dropped both selector keys as "deprecated training-only keys".Fix
Gr00tN1d7Processor.__init__takesuse_albumentationsandletter_box_transformexplicitly (defaultsFalse, same as Isaac-GR00T) instead of dropping them;save_pretrainedpersists them so save/load round-trips._build_eval_image_transformselects the recipe like Isaac's processor does:use_albumentations=True-> newAlbumentationsEvalImageTransform, a cv2/numpy transcription of Isaac-GR00T's eval branch and albumentations 1.4.18 (SmallestMaxSizerounding,FractionalCenterCroptruncation,LetterBoxPad), including thecrop_fraction/shortest_image_edgeNonefallbacks. No albumentations dependency;cv2is already there via vLLM'sopencv-python-headless, imported lazily inside the helpers likecosmos3/transfer.py.LetterBoxTransformonletter_box_transform, as Isaac-GR00T does (no-op for square frames)._get_vlm_inputs(HWC uint8 in,(C, H, W)uint8 tensor out) is unchanged, so the call site is untouched.Behavior change
use_albumentations: true(both public N1.7 checkpoints) now get the pixels they were trained on; outputs change (correctness fix).use_albumentations: falseor without the key keep the torchvision recipe; the only difference is that letterboxing now followsletter_box_transform(absent ->False, Isaac's default), which is a no-op for square frames.state_dropout_prob/extra_augmentation_configremain training-only and are still ignored.Not touched: #6568 / #6450 (pipeline_gr00t.py). #5028 changes the same file, but its
AutoProcessor.register(Gr00tN1d7Config, ...)fix is already on main via #5042.Test Plan
New CPU test
tests/diffusion/models/gr00t/test_processing_eval_image_transform.py(core_model+cpu, runs in "Simple · Diffusion Test"):Nonefallbacks, geometry/rounding rules, contract errors, letterbox gating on both recipes,_get_vlm_inputsend-to-end with the new transform,save_pretrained->from_pretrainedround-trip, legacy configs without the flags;(C, H, W)uint8 output of Isaac-GR00T's own eval transform (build_image_transformations_albumentations(...)[1]throughapply_with_replay, Isaac-GR00T 51d4c89 / albumentations 1.4.18 / opencv 5.0.0.93) on 4 deterministic integer-generated frames x letterbox on/off. The non-square frame is 512x768 so the INTER_AREA downscales are integer factors (exact integer sums in OpenCV): the goldens do not depend on the CPU architecture. Generating them also cross-checked Isaac-GR00T, this branch's code and an independent transcription for bitwise agreement on every case, and the torchvision branch against Isaac's torchvision eval transform. Regenerating a digest needs Isaac-GR00T installed, whose transformers pin conflicts with this repo's, so it cannot run in CI; the test module's docstring states the procedure.pytest -sv tests/diffusion/models/gr00t -m 'core_model and cpu' pre-commit run --all-filesProcessor-level parity (Isaac-GR00T processor vs patched processor, transformers 4.57.6 environment, DROID checkpoint, 2 seeds x batch 1/2):
pixel_values,input_ids,image_grid_thw,attention_mask,state,backbone_features,absolute_actionall bitwise equal, also through the real serving path.vLLM Version: 0.28.0
vLLM-Omni Commit: 10701e1 (upstream/main, branched here)
Test Result
(17 new cases plus the 4 pre-existing pipeline tests. On main the new module fails to import
AlbumentationsEvalImageTransform; against main's recipe the pinned Isaac-GR00T goldens differ byup to 255 gray levels on the checkerboard frame, 247 on the noise frame, and the non-square frame
even differs in shape, 256x256 instead of 256x383.)
End to end on GB200
Isaac-GR00T pins
transformers==4.57.3/torch==2.9.0while vLLM-Omni main needs transformers 5.x, so the two cannot share an interpreter. They were run as two processes on one GB200 node againstnvidia/GR00T-N1.7-DROID, exchanging tensors through disk: the same observation file (one non-zero 256x256 frame per view — a zero frame is a fixed point of every resize/crop recipe and would show nothing), and the reference's own initial denoising noise captured and injected into the vLLM-Omni run. The serving side drivesGr00tPolicy.get_actionand imports nothing butvllm_omni. Both arms use the same reference dump and differ only bygit checkout:pixel_valuesinput_ids,attention_mask,image_grid_thw,state,embodiment_id,state_featuresbackbone_featuresaction_pred(normalized)eef_9d/gripper_position/joint_positionThe processor reports its recipe as
Composeon main andAlbumentationsEvalImageTransformon this branch, so the switch is observable rather than inferred.pixel_values— the tensor this PR is about — goes to bitwise equality. The downstream rows do not, and should not be read as if they could: the two sides necessarily run different torch/transformers builds, and that difference alone is worth roughly the residual in the right-hand column. Both arms carry that same floor, so the honest claim is the contrast between the columns, not the absolute value of either. A same-process end-to-end bitwise check is not reachable while the dependency pins conflict.