Skip to content

fix(inference): warn when a transfer setting cannot take effect - #338

Open
rwagwani wants to merge 1 commit into
NVIDIA:mainfrom
rwagwani:fix/transfer-ignored-setting-warnings
Open

rwagwani wants to merge 1 commit into
NVIDIA:mainfrom
rwagwani:fix/transfer-ignored-setting-warnings

Conversation

@rwagwani

@rwagwani rwagwani commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

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

@rwagwani
rwagwani force-pushed the fix/transfer-ignored-setting-warnings branch from b8d2948 to 0ab0bea Compare October 30, 2026 06:55
@yaoxu-crypto yaoxu-crypto added the Skill New skill or updating existing skill label Oct 30, 2026
@lfengad

lfengad commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the additional Transfer diagnostics. I reviewed the incremental change on top of #335 and did not find a blocking implementation issue in this pass. This should follow #335 rather than merge ahead of it, since the current diff includes that PR as well. Once #335 is merged, could you please update this branch so the remaining diff is isolated, then satisfy the current-main checks and required approvals? The earlier ValueError for already-invalid weights and the private request-field/equality change are intentional behavior differences, not just message edits; please retain those explicit notes in the description.

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. Make the code say what the docs already say. The
shipped cookbook specs and the Sim-Transfer skill requests build exactly
as before, with no new warnings; new L0 tests that run in CI build them
all through build_sample.

Warnings only (the generated video does not change):
- a single-control `weight` (weights are normalised by their sum, and
  the causal Sim-Transfer path hard-codes 1.0);
- on single-view distilled causal Transfer, a table of fields the
  fixed-step sampler never reads (negative prompt, guidance, sigma_max,
  shift, normalize_cfg, num_conditional_frames and both guidance
  windows), a chunk size unlike num_frames, and model_mode other than
  video2video; and `--sampler` other than unipc on any fixed-step
  checkpoint;
- a reversed guidance_interval / control_guidance_interval (lo >= hi)
  where that guidance pass would otherwise run;
- on single-view Transfer, an explicitly requested fps or num_frames
  that differs from what the source video gives, checked in
  generate_transfer_sample (and the streaming path) against the resolved
  length and frame rate; and prompt_upsampling=True;
- both negative_prompt and negative_prompt_file set explicitly.

Error: on the 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.

A built OmniSampleArgs now records which fields the request set (a
private attribute, not serialized), so generation can tell a request
from a default. Pydantic includes private attributes in ==, so
test_sample_args compares model_dump() instead.

Signed-off-by: rwagwani <rwagwani@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rwagwani
rwagwani force-pushed the fix/transfer-ignored-setting-warnings branch from 0ab0bea to 4c9830f Compare October 31, 2026 17:22

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skill New skill or updating existing skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants