Skip to content

Cosmos3: give the generation positions a contiguous tensor so a new prompt length does not recompile the denoise step - #922

Merged
NSagan271 merged 2 commits into
mainfrom
users/atindra/cosmos3-prompt-buckets
Oct 28, 2026
Merged

NSagan271 merged 2 commits into
mainfrom
users/atindra/cosmos3-prompt-buckets

Conversation

@merceod

@merceod merceod commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

The Cosmos3 policy's compiled denoise_step (dynamic=False) recompiled for every new prompt length: 2.2-3.4 s on that request with a cold Inductor cache, 0.5-0.6 s warm, and a busy server walking toward the 84-recompile limit. TORCH_LOGS=recompiles shows each new prompt failing one guard, the stride of position_ids_cond: the action and sound paths pass position_ids[:, und_len:], a view whose row stride is the prompt plus generation length, while the text K/V only enters after the attention graph break.

The static inputs now carry gen_position_ids, a contiguous copy made once per request in packing.py, used at every call site. The video paths already passed their own tensor. No padding or bucketing needed.

Numbers, 2x B300, 8 droid_100 observations with 8 distinct prompt lengths (6-22 tokens), seeds 1000+i:

  • Recompiles per log: main 23 (one per new prompt), this branch 15, all at startup.
  • First request on a new prompt minus its repeat: main +2190 to +3405 ms cold, +518 to +608 warm; this branch -91 to +47 cold, -49 to +38 warm.
  • Eager main vs eager branch: bit-identical 8/8. Compiled vs eager: 1.4% / 2.2% median / max rel. L2 (main 1.6% / 2.5%).

Tests: a CPU test that the generation band has the same size, stride and offset for 4, 9 and 23 prompt tokens (fails on main). mstar/model/cosmos3/tests, test_cosmos3_edge_model.py, test_cosmos3_graph_switch.py: 217 passed, 21 skipped (main 216 / 21). e2e --tier full cosmos3_edge_droid and cosmos3_nano_ar pass on B300 with the flashinfer configs. ruff clean.

Action and sound requests passed position_ids[:, und_len:] to the
compiled denoise step. That view's stride is the prompt length plus the
generation length, and the step is compiled with dynamic=False, so every
new prompt length failed the stride guard and recompiled it: 2-3 s on the
first request with a cold Inductor cache. The static inputs now carry a
contiguous copy, made once per request, and a new prompt length replays
the step compiled at startup.
@mstar-review

mstar-review Bot commented Oct 28, 2026

Copy link
Copy Markdown

Automated review

🟡 No findings on 0556690, light review

Description

  • The latency and recompile numbers don't give the command or harness that produced them, so they can't be reproduced. Missing: how the server was launched, how requests were sent, and how 'first request minus its repeat' was measured.
  • AGENTS.md asks for the torch/CUDA build alongside the GPU hardware for the B300 e2e and parity runs, and the description doesn't give it.
What the review checked
  • Both static-input builders now set gen_position_ids: build_static_inputs, which also covers build_t2i_static_inputs through its wrapper, and build_action_static_inputs. In submodules.py, the request statics come only from _build_static and _build_action_static, so every dict that reaches a denoise call site carries the key.
  • No position_ids[:, und_len:] slice is left in mstar/model/cosmos3/ outside test helpers, so every action, sound, batched-CFG and batched-request call site uses the contiguous copy.
  • position_ids is never reassigned or changed in place after packing, so gen_position_ids can't fall out of sync with it. The other position_ids writes in submodules.py belong to the reasoner's own inputs, not the DiT statics.
  • Cond and uncond prompts have different lengths, so their slices used to have different strides; both now pass contiguous tensors with the same layout.
  • .contiguous() runs once per request at packing time, not per denoise step, so it adds no per-step CPU or GPU cost (AGENTS.md invariant 6).
  • The new test in mstar/model/cosmos3/tests/test_action.py is CPU-only, runs under the CPU Core paths and checks that the values equal the old slice.

Advisory, not a merge gate. Rules: AGENTS.md. Reply to discuss.

@NSagan271 NSagan271 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending some live testing that I'm running

@NSagan271

Copy link
Copy Markdown
Collaborator

🤖 Automated live test, AI-generated (Claude Code). Not checked line by line by a human. Numbers come from one allocation, so treat small deltas as provisional.

Live test: #922 Cosmos3 generation position ids

Head 05566900 vs its base b47e35b3 (no Cosmos3 changes on main since). cosmos3_edge_droid, 1× H100 80GB, SHM, Rust runtime, TORCH_LOGS=recompiles. 15 DROID prompts of distinct lengths (3–47 words), each sent once and then repeated, one droid_100 frame per request, seeds 1000+i. Builds alternated; the Inductor cache was cold on 3 boots per build and warm on 2–3. Video+sound used cosmos3_nano.

Verdict: ✅ A new prompt length no longer recompiles the denoise step, and eager outputs are bit-identical to base.

Claim Result Evidence
A new prompt length doesn't recompile ✅ Recompiles after startup: base 12 on every one of 6 boots, all from position_ids_cond stride mismatch at index 0 in denoise_step_batched_cfg; PR 0 on all 6. Totals including startup: base 27, PR 15
No first-request penalty ✅ First request minus its repeat, median per boot. Cold cache: base +1,604 / +1,869 / +1,886 ms (max +2,359), PR −16 / +9 / +35 ms. Warm cache: base +388 / +421 ms (max +838), PR −10 / +3 / +16 ms
Eager output unchanged ✅ TORCH_COMPILE_DISABLE=1: PR vs base 15/15 bit-identical, the same as base vs base and PR vs PR
Batched action path (_forward_batched_action) ✅ 4 concurrent policy requests, 15 lengths × 2: recompiles after startup, base 8 / 5, PR 0 / 0. 30/30 returned 200 on every boot
Video + sound path (_forward_video_sound_gen) ✅ cosmos3_nano, generate_sound, 5 prompts of distinct lengths × 2, 29 frames at 256×256, 4 steps. Base hits the same position_ids_cond stride guard 10 times per boot, PR 0. A new prompt length's first request minus its repeat: base +399 to +450 ms, PR +39 to +87 ms. Eager: video and audio bit-identical to base, 10/10 each. Compiled: identical within each build across boots, different between builds
Compiled close to eager (1.4% / 2.2% on B300) ➖ not reproduced on H100 for either build Compiled vs eager, median / max relative L2 over the 15 actions. PR: 5.1% / 10.7% on all 4 caches (identical across them). Base: 1.5% / 31%, 7.7% / 23%, 15.5% / 21%, 15.5% / 32%, depending on the cache. Base compiles a separate graph per prompt length, and its error varies with which kernels each compile picked; the PR's single graph is stable and its worst case is lower

Other

  • Latency once warm is unchanged: repeat median 162–214 ms on both builds. Every request returned 200, and no server logged an error.
  • Base's error on H100 (up to 15% median, 32% max, compiled vs eager) is larger than the description's B300 figures and comes from the existing compile, not this PR. It may be worth its own look.
  • Not tested: B300/B200; the batched-sound path (_forward_batched_sound), which these runs, at concurrency 1, didn't reach.

@NSagan271
NSagan271 merged commit d4b02b6 into main Oct 28, 2026
10 checks passed
@NSagan271
NSagan271 deleted the users/atindra/cosmos3-prompt-buckets branch October 28, 2026 01:17
dghosef added a commit that referenced this pull request Oct 28, 2026
Main since 73c6c17: #883 mstar_rust in the Linux wheels, #901 output
spans (BatchedModelOutput.output_spans, NodeSubmodule.spanned_outputs, the
worker's crop), #904 DiT seed noise drawn at ingest (on_request_added,
NoiseStager deleted, to_device_nonblocking as one memcpy into pinned memory),
#903 API-server torch pool from the CPU quota and pooled image encodes,
#924 robotics harness, #819 Inductor cache under ~/.cache, #881
CommGroup.agree / JointGroups.agree (agree_across_ranks and
flagged_on_any_rank deleted), #724 CUDA IPC KV in one 2D memcpy a page,
#387 GLM-5.2 (MLA, fp8 MoE with NoAuxTcRouter, the DSA resource), #921
vjepa2 AC crop, #922 Cosmos3 contiguous positions, #923 dense attention
falls back when fa3-fwd has no kernel for the GPU.

Conflicts:
- mstar/utils/h2d.py, to_device_nonblocking: the stack's signature
  (data: Tensor | Sequence, device, dtype=None) and DeviceCapability
  pin_memory gate, with main's tensor path (#904): a CPU tensor is written
  into a fresh pinned buffer with one memcpy rather than pin_memory()'s
  copy_ over the intra-op pool. Every CPU tensor now goes through a fresh
  buffer, so the stack's "an already-pinned source is read in place"
  caveat (R3.3) no longer applies and is dropped from the docstring.
  test_accelerator_graph_backend's pinning test checks the pinned
  torch.empty and its non-blocking copy instead of pin_memory().
- mstar/model/components/diffusion/denoise_loop.py: imports, both
  (main's copy, the stack's functools).

Main code on a superseded path, ported here:
- GLM-5.2 (glm52/submodules.py): its own check_stop / check_stop_batched
  (four arguments, step_walk, its own row-map loop) / _stops ->
  token_stop = TokenStop(decode_loop, new_token, eos_token_ids, sampler)
  with a _token_stops override for its two differences (a stop token at
  prefill ends the reply at the first decode step; max_tokens counts the
  prefill's token), and a check_stop override for that closing step,
  whose token postprocess drops. test_glm52_model's helper and
  test_model_kv_layouts' GLM cap test build the real submodule (over a stub
  language model) instead of object.__new__, which would leave token_stop
  unset.
- PinnedStager.to_device (#387), a second "host values -> new device
  tensor" helper beside to_device_nonblocking: deleted with its test
  (test_pinned_stager_to_device.py); its callers, GLM-5.2's prefill
  seq_lens and the DSA resource's page tables, call
  to_device_nonblocking(list, device, dtype=torch.long).
  test_dsa_resource spies on that instead.

The other files both sides touched (engine.py, accelerator_graph_runner.py,
kv/manager.py, attn/base.py, ragged/base.py, communication.py,
model/base.py, submodule_base.py, entrypoint.py, conductor.py, moe.py,
image_io.py, vjepa2_model.py, docs, tests) auto-merged into disjoint
hunks: per file the merged delta equals main's own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants