Repository navigation
fix(inference): name the cause in Transfer rejection messages - #335
Merged
Merged
Conversation
Three Transfer rejections were correct but did not say what the request
did wrong:
- The direct causal Transfer check listed all four rules ("exactly one
control hint, num_first_chunk_conditional_frames=0,
control_guidance=1.0, and share_vision_temporal_positions=true"). It
now names only the rules this request breaks, with the values passed.
- On the 480-only SF-DMD checkpoint, 256/704/768 were told "480 and 720
tiers only", so a user tried 720 and only then learned "480 only".
The direct-resize check now raises the checkpoint's own limit, using
the same predicate as its resolution contract, and both messages
include the rejected value.
- The direct_resize aspect error ("causes N% aspect distortion") did
not mention a forced aspect_ratio. It now names it, says direct_resize
does not crop, and names the bucket that would fit, if any.
The Transfer cookbook's multi_control.json stated fps 30 / num_frames
121 for a 16 fps, 93-frame video. Neither value is read for a video
input, so the spec now states the real values without changing its
output, and the README field reference says fps/num_frames should match
the video.
Signed-off-by: rwagwani <rwagwani@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rwagwani
force-pushed
the
fix/transfer-error-messages
branch
from
October 30, 2026 06:55
7e7f1ad to
bc48e94
Compare
lfengad
approved these changes
Oct 31, 2026
lfengad
left a comment
lfengad
left a comment
Collaborator
There was a problem hiding this comment.
Static review of bc48e94: the accepted/rejected request conditions are preserved while the error messages identify the actual cause; the cookbook timing values are aligned with the input video. I found no blocking issue in this pass. Existing checks on this head have passed. The changed paths are outside the current I4 mapping. Approving the code review; merge still needs the remaining required approval and an up-to-date branch under the normal main protection rules. No tests were run locally for this review.
lfengad
pushed a commit
that referenced
this pull request
Nov 2, 2026
Stacked on #335 (rejection messages and the cookbook spec fix). This PR's own change is the last commit. ## Summary Several transfer settings are accepted without any message in setups where the code never reads them, so users change them and get an unchanged video. Found while validating every parameter of Cosmos3-Nano-Sim-Transfer (checkpoint 23b77384) and Cosmos3-Nano on AV and Robotics inputs. Most of these limits are already in the docs; this PR makes the code say them at run time. It changes no generated video, and every shipped cookbook spec and Sim-Transfer skill request builds exactly as before, with no new warnings. ## Warnings (no output change) - A single-control `weight`: weights are normalised by their sum, and the causal Sim-Transfer path hard-codes 1.0. - Single-view distilled causal Transfer: a table (`_DISTILLED_TRANSFER_IGNORED_FIELDS`) of fields the fixed-step sampler never reads (negative prompt, guidance, sigma_max, shift, normalize_cfg, num_conditional_frames, both guidance windows). It warns only when the sample or CLI sets a non-neutral value, not for values from a defaults file. Also a chunk size unlike `num_frames`, and `model_mode` other than `video2video`. - `--sampler` other than unipc on any fixed-step checkpoint (checked once at model load). - A reversed `guidance_interval` / `control_guidance_interval` (`lo >= hi`), only where that guidance pass would otherwise run. - Single-view Transfer: a requested `fps` or `num_frames` that differs from what the source video gives. This is checked in `generate_transfer_sample` (and the streaming path), against the length and frame rate generation actually resolves, so there is no second copy of that logic. Also `prompt_upsampling=True`, and both `negative_prompt` and `negative_prompt_file` set explicitly. ## Error - Bidirectional single-view path: negative or all-zero control weights raise `ValueError` when the sample is built, instead of a bare `assert` inside `build_transfer_batch`. The causal path and joint camera+LiDAR transfer do not read these weights, so they are left alone. ## Behaviour changes, stated plainly - The only new error is the control-weight `ValueError` above. Those requests already failed, later, with a bare `AssertionError` after model load. - A built `OmniSampleArgs` now records which fields the request set (a pydantic private attribute, `requested_fields`, not serialized), so generation can tell a request from a default. Pydantic includes private attributes in `==`, so `test_sample_args` compares `model_dump()`. No production code compares samples with `==`. ## Not changed here (follow-up) On normal Nano/Super, the built-in video2video defaults always supply a `negative_prompt`, so a request-level `negative_prompt_file` is never read, and a missing path is accepted. The cookbook specs set `../assets/negative_prompt.json`, whose caption differs slightly from `defaults/neg_prompts.json`. Fixing this changes cookbook output (precedence, and resolving the path relative to the spec), so it should be its own PR. ## Verification - New L0 tests (`transfer_shipped_specs_test.py`, plus additions to existing files). They build all six cookbook specs and the six official Sim-Transfer skill requests (written by `prepare_hf_sample.py`, run with the skill's defaults file and CLI flags) through `from_files` + `build_sample`, and assert no errors and no `[transfer]` warnings. They do not need the Git LFS videos, so they run in CI. A separate spec-vs-video check (fps/num_frames match the asset) runs only where the LFS videos are checked out. - Touched test files: 384 passed, 1 skipped. - The CI unit-test job (`-m L0`, same command as `gpu-tests.yml`), replicated on a single GB300: running; I will add the result here. - GPU, Cosmos3-Nano-Sim-Transfer with the skill command (official example2 depth and edge requests) and Cosmos3-Nano with the cookbook depth spec: no new warnings, and the same videos as main within same-seed run-to-run noise. A deliberately mis-set Sim-Transfer request (`depth.weight=5`, `sigma_max=200`, `fps=10` on a 30 fps video) prints exactly those three warnings and produces the same video as on main. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: rwagwani <rwagwani@nvidia.com> Co-authored-by: Claude Opus 5.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.
Summary
Three Transfer rejections are correct but don't say what the request did wrong. This PR makes each message name the cause. Only message text changes: every request that runs today still runs, and every request that is rejected today is still rejected.
Found while validating every parameter of Cosmos3-Nano-Sim-Transfer (checkpoint 23b77384) on AV and Robotics inputs.
Changes
Direct causal Transfer requires exactly one control hint, num_first_chunk_conditional_frames=0, control_guidance=1.0, and share_vision_temporal_positions=trueIt now names only the rules this request breaks, with the values passed:
_is_sf_dmd_transfer_model, factored out of_apply_sf_dmd_transfer_inference_contract). Both messages now include the rejected value:This SF-DMD Transfer checkpoint supports only resolution='480', got resolution='768'.Other models keep the existing "480 and 720 tiers" message.aspect_ratiowithdirect_resize.Unsupported Transfer sample: 1280x720 -> 544x736 causes 140.52% aspect distortion, exceeding 3%does not mention that the user's ownaspect_ratiocaused it. The message now names it and saysdirect_resizedoes not crop. It also names the bucket automatic selection would pick, if one fits within 3%.multi_control.json. It statedfps: 30,num_frames: 121for a 16 fps, 93-frame video (robot_pouring.mp4). For a video input, Cosmos Framework takes both from the video, so the spec now states the real values and its output does not change: on GPU, the old and new spec give 16 fps / 93 frames and the same video, within same-seed run-to-run noise. The Diffusers notebook does not use this spec. The README field reference and table now sayfps/num_framesshould match the video.Verification
transfer_direct_resize_test.py(the culprit messages),transfer_preprocessing_test.py(the aspect messages).sim_transfer_args_test.py: the 480-only message for every rejected tier, and the teacher-tier message kept on a bidirectional model that records 480.transfer_cookbook_specs_test.py: each cookbook spec'sfps/num_framesmatches its video. It runs where the Git LFS videos are checked out and skips otherwise (CI's unit-test checkout does not fetch LFS).transfer_test.pyandinference_test.py: 315 passed, 1 skipped (the spec that downloads its input).A follow-up PR (stacked on this one) adds run-time warnings for transfer settings that cannot take effect.
🤖 Generated with Claude Code