diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 53a7287b8..fd6abcc0c 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -240,6 +240,8 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#907](https://github.com/mudler/vllm.cpp/issues/907) | `BACKEND-CUDA-COMP-CORE` | Five tests are red on dgx.casa (GB10, sm_121a) at `main`, proven PRE-EXISTING by a control build at `10b8bbdaa`: `test_capi` (SIGSEGV in an ABI v8 custom-logits-processor case, plausibly [#547](https://github.com/mudler/vllm.cpp/issues/547) or [#844](https://github.com/mudler/vllm.cpp/issues/844)), `test_cuda_ops` 439/440, `test_linear_method` 83/85, `test_ops_gdn` 4899/4900 ([#614](https://github.com/mudler/vllm.cpp/issues/614)), `test_qwen3_5_gdn_spec_routing` 119/123. Three of the five had no issue at all, which is why this exists | bug | | [#912](https://github.com/mudler/vllm.cpp/issues/912) | `ENG-EXPERT-STREAM` | Stream routed experts from NVMe so a model larger than device memory runs. Target `Qwen/Qwen3.8-2.4T-A95B`, REGISTERED against `Qwen3_5MoeForCausalLM` and blocked only on capacity: 370 GiB at UD-Q1_0 against 128 GB of unified memory on GB10. The only one of the three offload rows that helps on a unified-memory host, because `ENG-WEIGHT-OFFLOAD` and `ENG-HYBRID-PLACEMENT` both move bytes inside one physical pool. Cheaper than the spec assumed: on the GGUF path the mmap'd file already IS the bank and the per-expert slicer landed 2026-07-22 ([#824](https://github.com/mudler/vllm.cpp/issues/824)) | feature | | [#911](https://github.com/mudler/vllm.cpp/issues/911) | — | A `file:line` anchor into a file the row is ITSELF editing is stale by default, and spec BODIES are checked by nothing: `check-agent-record.py`'s `MATRIX_PATHS` (`:521`, `:529-530`) covers the five matrices, `feature-matrix.md` and `specs/model-family-inventory.md`, leaving 4772 line-carrying citations across 315 `.agents/specs/*.md` unexamined (positive control: 2314 line-less `.cpp` mentions match the same shape). `ltx25-prompt-adaln.md` shipped EIGHT stale repo-local anchors across two repair commits, moved by its own `020381676` and by `98f8e046d` (#658), then SEVEN more that were correct at `00613767d` and wrong at the merge of `origin/main`, because `0785cfc4d` (#882) added 70 lines to `ltx2_video.cpp` and 306 to `test_ltx2_video.cpp` ahead of every one. The obvious checker is a TAUTOLOGY — reading the span out of the file it validates reports 27 of 27 fresh on the same tree where reading the spans against their CLAIMS finds seven stale. Remedy is already in use and unwritten: `path:NN @ ` for a historical claim, claim-sourced uniqueness re-derivation for a live one, re-run after the merge. Narrower than [#632](https://github.com/mudler/vllm.cpp/issues/632) on surface and sharper on mechanism. Listed under `## Owed` in [`ltx25-prompt-adaln.md`](specs/ltx25-prompt-adaln.md) | bug | +| [#919](https://github.com/mudler/vllm.cpp/issues/919) | `LTX25-RESOLUTION-ENVELOPE` | `vllm_video_generate` integer-divides `width`/`height` into the latent grid (`ltx2_video.cpp:1456-1463 @ 5a0ffe9e3`) with no divisibility check, so a 100x100 request on the distilled two-stage recipe silently renders 96x96. The only geometry guard in the LTX path is a LOWER bound (`ltx2_video.cpp:1464-1471 @ 5a0ffe9e3`). Every repo-local anchor in this row is SHA-pinned because the fix edits the files it cites, and inserts lines above both spans (#911). Upstream hard-validates and raises at the top of a pipeline `__call__` — `assert_resolution` (`ltx-pipelines utils/helpers.py:540-551` @ `fd4ded7f2`), 64 for two-stage and 32 for one-stage, NINE invocations including `ti2vid_two_stages.py:184` and `ti2vid_two_stages_hq.py:199` — so mirroring means refusing, not flooring. Nine, not the 21 lines a grep for the name returns (9 invocations + 1 definition + 10 imports + 1 `__all__` string), and not every pipeline: 13 pipeline `__call__`s take a resolution and the three `*_mgpu.py` variants plus `hdr_ic_lora.py:352` skip the guard. `docs/USAGE.md:626-629 @ 5a0ffe9e3` already documents the rule as though it were enforced. Frames are the OPPOSITE answer: upstream floors an explicit `num_frames` exactly as we do (`ltx_core/types.py:113`) and validates it nowhere, so that half is a doc correction | bug | +| [#921](https://github.com/mudler/vllm.cpp/issues/921) | — | The res_2s DENOISING LOOP (`ltx-pipelines utils/samplers.py:206-447` @ `fd4ded7f2`) is unported, so `TI2VidTwoStagesHQPipeline` cannot be served. What exists is one substep's SDE arithmetic: `Ltx2Res2sSdeCoeff`/`Ltx2Res2sStep` (`ltx2_pipeline.cpp:307-360 @ 5a0ffe9e3`, the two functions in full) mirror `Res2sDiffusionStep` (`diffusion_steps.py:118-190`) and are gated. Absent are the `phi`/`get_res2s_coefficients` exponential integrator (`utils/res2s.py:4-62`), the SECOND transformer evaluation per step at `sub_sigma = sqrt(sigma * sigma_next)` (`samplers.py:315` and `samplers.py:380-386`, spelt out because a bare `:NN` after a res2s.py citation reads as res2s.py) against our once-per-step loop (`ltx2_video.cpp:1735 @ 5a0ffe9e3`, with its single forward at `ltx2_video.cpp:1813-1817 @ 5a0ffe9e3`), the bong anchor refinement (`samplers.py:357-364`), and any `Ltx2StepperKind` enumerator to select it. The sampler IS the HQ variant, so this arm must refuse by name rather than substitute Euler and render something plausible that is quietly not HQ. Listed under `## Owed` in [`ltx25-resolution-envelope.md`](specs/ltx25-resolution-envelope.md) | feature | | [#922](https://github.com/mudler/vllm.cpp/issues/922) | `LTX25-A2V-AUDIO-INPUT` | LTX-2.5 audio-to-video (`A2VidPipelineTwoStage`, `a2vid_two_stage.py:53,143`) is absent: `vllm_video_params` carries no field or extra that accepts a driving waveform and `ltx2-gen` has no `--audio-path`, so nothing turns a file on disk into the audio latent the DiT's audio stream consumes. Distinct from reference-audio conditioning, which is correctly refused by name at `ltx2_video.cpp:1348-1355 @ 5a0ffe9e`; the two share one blocking dependency, the audio VAE ENCODER load path (`ltx2_loader.cpp:1295-1300` materializes `audio_vae.decoder.` only). The analysis half is already ported and unreached — `Ltx2AudioEncoderForward` (`ltx2_audio_vae.cpp:1114`), `Ltx2WaveformToLogMel` (`:1019`), `Ltx2SlaneyMelFilterbank` (`:970`) — and the engine applies ONE `phase.noise_scale` to both streams (`ltx2_video.cpp:1708-1712 @ 5a0ffe9e`) where upstream's `ModalitySpec` carries `noise_scale` and `frozen` per modality (`utils/types.py:99-112`). Spec [`ltx25-a2v-audio-input.md`](specs/ltx25-a2v-audio-input.md) | feature | | [#924](https://github.com/mudler/vllm.cpp/issues/924) | — | LTX-2.5 `RetakePipeline` (`retake.py:53,151`) regenerates a chosen time region of an existing video and is not served; the recipe table refuses the kind by name at `ltx2_pipeline.cpp:1131-1136`, which is the correct current state. Judged a SEPARATE row from audio-to-video (#922) after reading both upstream files: they share the audio VAE encoder and nothing else. Retake needs `TemporalRegionMask` (`noise_mask_cond.py:10-47`, zero hits in this tree, two coordinate conventions), video-file ingestion plus video VAE encode (`helpers.py:165-233`), and the audio VAE DECODER on the output side, where A2Vid returns the caller's waveform untouched (`a2vid_two_stage.py:301-303`). Their latent-length policies also disagree — A2Vid truncates only (`a2vid_two_stage.py:202`), Retake truncates or zero-pads (`helpers.py:149-162`) — so one shared helper would be wrong for one of them. Listed under `## Owed` in [`ltx25-a2v-audio-input.md`](specs/ltx25-a2v-audio-input.md) | feature | | [#927](https://github.com/mudler/vllm.cpp/issues/927) | — | LTX-2.5 accepts and NEVER READS seven typed fields that MiniMax-H3 reads: `audio_vae_config_path`, `video_vae_config_path`, `tokenizer_path`, `encoder_max_layers` on the load side, and `flow_shift`, `audio_flow_shift`, `task` per generation. `CheckKnownExtras` / `CheckUnservedExtras` (`ltx2_video.cpp:314-356 @ 5a0ffe9e`) exist to refuse an unserved knob by name and cover the `extras` MAPS only, never the typed fields. Sharpest case: `docs/USAGE.md` shows an H3-shaped recipe passing `--audio-vae-config`, and against LTX-2.5 that file is accepted and never opened, because the engine takes the config from the checkpoint `__metadata__` (`ltx2_video.cpp:920-921 @ 5a0ffe9e`) — so a JSON that disagrees is silently overridden. `audio_flow_shift` is even validated positive (`video_api.cpp:236`) before being dropped, which makes the request look served. Not fixed in flow because a blanket refusal needs a per-field serve/refuse/not-applicable decision, the distinction [#758](https://github.com/mudler/vllm.cpp/issues/758) records this project getting wrong. Found while surveying for [#922](https://github.com/mudler/vllm.cpp/issues/922). Listed under `## Owed` in [`ltx25-a2v-audio-input.md`](specs/ltx25-a2v-audio-input.md) §9 | bug | diff --git a/.agents/specs/ltx25-resolution-envelope.md b/.agents/specs/ltx25-resolution-envelope.md new file mode 100644 index 000000000..04224d3a0 --- /dev/null +++ b/.agents/specs/ltx25-resolution-envelope.md @@ -0,0 +1,535 @@ +# LTX-2.5 — the resolution envelope, and the refusal that makes it a contract + +Row: `LTX25-RESOLUTION-ENVELOPE`. Campaign: [`ltx-2-5.md`](ltx-2-5.md) +(operator-owned; **not edited by this row**). Issues: +[#919](https://github.com/mudler/vllm.cpp/issues/919) (the defect this row +fixes), [#921](https://github.com/mudler/vllm.cpp/issues/921) (the res_2s +denoising loop, listed under `## Owed` below). Sibling of +[#644](https://github.com/mudler/vllm.cpp/issues/644). + +Upstream pin: + +| Reference | Revision | +|---|---| +| Lightricks/LTX-2 (`packages/ltx-core`, `packages/ltx-pipelines`) | `fd4ded7f2d88d3da713abcdd4ad41ecc4a9314ca` | + +Verified at the local checkout `/home/mudler/_git/LTX-2`: +`git rev-parse HEAD` = `fd4ded7f2d88d3da713abcdd4ad41ecc4a9314ca`, clean tree. +Every anchor below was read from that checkout. + +--- + +## 0. Honesty statement — what this row does and does not claim + +The dispatching brief asked for the full `TI2VidTwoStagesPipeline` and +`TI2VidTwoStagesHQPipeline` port **and** the resolution story. A ground-truth +survey against the verified pin, relayed by the coordinator mid-row, narrowed +this row to the second half. This section records the narrowing before any work +starts so that no later reader mistakes the delivered slice for the whole ask. + +**This row delivers:** upstream's geometry constraint, mirrored as a refusal at +the production entry point, plus the published envelope. + +**This row does not deliver, and does not claim:** + +1. **The res_2s sampler.** `Ltx2Res2sStep` / `Ltx2Res2sSdeCoeff` + (`src/vllm/model_executor/models/ltx2_pipeline.cpp:307-345`) already mirror + `Res2sDiffusionStep` and are gated. That is one substep's SDE arithmetic, not + the sampler. The sampler is `res2s_audio_video_denoising_loop` + (`samplers.py:208-447`), and its exponential-integrator coefficients, its + second transformer evaluation per step, and its bong refinement are all + absent. §5 and #921. +2. **A raised resolution ceiling.** There is no code cap to lift. The only + geometry guard in the LTX path today is a **lower** bound + (`ltx2_video.cpp:1464-1471 @ 5a0ffe9e3` — after this row, the + `if (vshape.frames < 1 || vshape.height < 1 || vshape.width < 1)` refusal + inside the phase loop, which this row's guard now sits above). The real + ceiling is host memory and decode + throughput, it is already measured, and it is already **unattributed** — §4. + Attributing it needs the GPU, which this row must not use. + +## 1. The question, and the evidence that answers it + +Nothing in this tree established what resolutions LTX-2.5 supports. The only +render ever performed was 9 frames at 128x128. `ltx2-gen` exposes `--width`, +`--height` and `--frames`, and exposing a flag is not supporting a value. + +### 1.1 What upstream supports, and what it enforces + +**Width and height are hard-validated, and upstream raises.** +`packages/ltx-pipelines/src/ltx_pipelines/utils/helpers.py:540-551`: + +```python +def assert_resolution(height: int, width: int, is_two_stage: bool) -> None: + """Assert that the resolution is divisible by the required divisor. + For two-stage pipelines, the resolution must be divisible by 64. + For one-stage pipelines, the resolution must be divisible by 32. + """ + divisor = 64 if is_two_stage else 32 + if height % divisor != 0 or width % divisor != 0: + raise ValueError(...) +``` + +It is not help text and it is not advisory. There are **nine invocations**, each +near the top of a pipeline's `__call__`, including the two this row's brief +named: `ti2vid_two_stages.py:184` (`is_two_stage=True`) and +`ti2vid_two_stages_hq.py:199` (`is_two_stage=True`), against +`ti2vid_one_stage.py:156` (`is_two_stage=False`). The CLI's `args.py` help text +carries the same rule, but the pipeline is what enforces it, and a caller who +reaches `__call__` from library code gets the `ValueError` either way. + +**Nine, and the other numbers in circulation are wrong.** `grep -rn +assert_resolution` at the pin returns **21** lines, and this row's first pass read +that as a call-site count in one place and as "ten" in another. Counted: + +| Kind | Count | Where | +|---|---|---| +| Invocations | **9** | `a2vid_two_stage.py:168`, `dfr_pipeline.py:291`, `distilled.py:213`, `dubit.py:212`, `ic_lora.py:229`, `keyframe_interpolation.py:170`, `ti2vid_one_stage.py:156`, `ti2vid_two_stages.py:184`, `ti2vid_two_stages_hq.py:199` | +| Definition | 1 | `utils/helpers.py:540` | +| Imports | 10 | one per calling module, plus `utils/__init__.py:12` | +| `__all__` string | 1 | `utils/__init__.py:44` | + +"Every pipeline's `__call__`" is wrong too. **13** pipeline `__call__`s take a +height and a width, and four of them never call the guard: +`distilled_mgpu.py:143`, `ti2vid_two_stages_mgpu.py:163`, +`ti2vid_two_stages_hq_mgpu.py:164` and `hdr_ic_lora.py:352`. (`retake.py:151` and +`t2a_one_stage.py:109` take no resolution at all and are excluded from the 13.) +So the guard is what every pipeline a caller reaches for a single-GPU +text/image-to-video render runs, and not a universal one — which changes nothing +about mirroring it, and does change what this document may claim. + +**Where 64 and 32 come from.** The VAE spatial factor is 32 +(`SpatioTemporalScaleFactors.default()` = `time=8, height=32, width=32`, +`ltx_core/types.py:31-33`). A one-stage pipeline runs at the requested size, so +its divisor is that factor. A two-stage pipeline runs stage 1 at `width // 2`, +`height // 2` (`ti2vid_two_stages.py:226-228`, `ti2vid_two_stages_hq.py:241-243`), +so the requested size must survive being halved and still divide the grid — +`32 * 2 = 64`. The divisor is the VAE factor times the worst spatial downscale +any phase applies. That is a derivation, not a coincidence, and §3 mirrors it as +one. + +**Frames are NOT validated.** An explicit `num_frames` passes through +`resolve_num_frames` (`utils/blocks.py:908-928`) untouched — the function returns +it verbatim when it is not an `AutoDuration` — and lands in +`VideoLatentShape.from_pixel_shape` (`ltx_core/types.py:108-123`), which floors: + +```python +frames = (shape.frames - 1) // scale_factors.time + 1 +height = shape.height // scale_factors.height +width = shape.width // scale_factors.width +``` + +`snap_frames_to_grid` (`utils/helpers.py:554-562`) encodes +`(frames - 1) % time == 0`, +and this row's first pass said it "is reached only from the AUTO-duration path". +**That premise is false**, and it was published in `docs/USAGE.md` and asserted +in a source comment before a fresh review caught it. There are three callers: + +| Caller | Path | +|---|---| +| `utils/helpers.py:581` | inside `seconds_to_clamped_num_frames` (`utils/helpers.py:565-585`) — the auto-duration path, as claimed | +| `dubit.py:215` | inside `DubitPipeline.__call__` (`:194-210`), three lines after its own `assert_resolution` at `:212` | +| `dubit.py:396` | the module's `main`, sizing the encoder's chunk count | + +**The decision it was used to justify survives, on a different and checkable +reason.** `DubitPipeline.__call__` (`dubit.py:194-210`) takes **no `num_frames` +parameter at all**; it reads a frame count from the reference video's container +metadata (`get_videostream_metadata`) and snaps that. Counted at the pin, it is +the only pipeline `__call__` that snaps and the only one with no `num_frames` +parameter — every `__call__` that *does* take one leaves it unsnapped, and the +six that route it through `resolve_num_frames` get it back verbatim +(`utils/blocks.py:920-921`). So no caller-supplied frame count is validated anywhere +upstream, which is the claim §3.2 actually needs. It floors, exactly as we do. + +**Upstream's own defaults**, which are the scale the envelope is measured +against (`utils/constants.py`): + +| Preset | Stage 1 | Stage 2 (output) | Frames | Steps | +|---|---|---|---|---| +| `PipelineParams` / `LTX_2_PARAMS` (`:42-76`) | 512x768 | **1024x1536** | 121 | 40 | +| `LTX_2_3_PARAMS` (`:83-88`) | 512x768 | 1024x1536 | 121 | 30 | +| `LTX_2_3_HQ_PARAMS` (`:95-98`) | 544x960 | **1088x1920** | 121 | 15 | + +`stage_2_height` / `stage_2_width` are `stage_1 * 2` (`:70-76`). All three +outputs are multiples of 64, as `assert_resolution` requires of a two-stage call. + +### 1.2 What our code assumes, and what actually binds + +Every repo-local anchor in this section is pinned `@ 5a0ffe9e3`, this row's base +SHA, because the row edits the file it cites and an unpinned anchor into a file +you are yourself moving is stale by default (#911). + +**The post-change positions are named by their code, not by a line number**, and +that spelling is itself a repair. The first pass gave them as +`ltx2_video.cpp:1485-1493` and `ltx2_video.cpp:1494-1501`; they were +`ltx2_video.cpp:1484-1492` and `ltx2_video.cpp:1493-1500` on that tree, and then +`origin/main` moved for #935 and put both of them past `ltx2_video.cpp:1600`. A +post-change +line number in a file the row is editing goes stale twice over — once from the +row's own insertions and again from every merge before it lands — so this section +states the expression instead, which the reader can find with one grep and which +no merge can move. + +`vllm_video_generate` resolves geometry at +`src/vllm/multimodal/ltx2_video.cpp:1401-1422 @ 5a0ffe9e3` and turns it into a +latent grid at `:1455-1463 @ 5a0ffe9e3` — after this row, the `phase_h`/`phase_w` +divisions and the three `vshape.*` assignments at the head of the phase loop: + +```cpp +const int64_t phase_h = height / phase.spatial_downscale; +const int64_t phase_w = width / phase.spatial_downscale; +... +vshape.frames = (frames - 1) / factors.time + 1; +vshape.height = phase_h / factors.height; +vshape.width = phase_w / factors.width; +``` + +Three binding constraints, and only one of them is a check: + +1. **A lower bound**, `:1464-1471 @ 5a0ffe9e3` — after this row, the + `if (vshape.frames < 1 || vshape.height < 1 || vshape.width < 1)` refusal + immediately below that block. The request must reach one latent cell. Present, + correct, and gated. It is also where a caller sent to a size of 0 by the old + suggestion wording landed (§3.1). +2. **`frames < 1`**, `:1422 @ 5a0ffe9e3`. Present. +3. **Divisibility — absent.** Integer division is the whole of it. + +**MEASURED, not reasoned.** The defect has two faces, and which one a request +gets depends on whether its floor is consistent across the phases. Both were +observed on the reduced fixture at `5a0ffe9e3`, before the guard existed: + +| Request | Recipe | What happened | +|---|---|---| +| width 80 | distilled two-stage | rendered **64x64**, exit success | +| width 100 | one-stage | rendered **96x64**, exit success | +| width 96 | distilled two-stage | threw `the upsampled latent is 4x2x2x2 but phase 'refine' needs 4x2x2x3` | + +The third is not a silent floor but it is not a usable error either: stage 1 +floors 48 to one latent cell while stage 2 needs three, so the upsampler's shape +check fires with a true statement about latents and no mention of the width the +caller passed. One guard at the entry point closes both faces, which is why the +test case carries all three sizes. + +`docs/USAGE.md:626-629 @ 5a0ffe9e3` already documents the divide-by-64 rule as +though something enforced it. Nothing does. That gap between a published promise +and the tree is the defect (#919), and it is the one thing in this story that is +a source-and-refusal question rather than a measurement one. + +## 2. Scope + +**In.** + +- `Ltx2AssertResolution`, mirroring `helpers.py:540-551`, in the pipeline + header/TU beside the other mirrored pipeline helpers. +- Its call at the geometry resolution in `ltx2_video.cpp`, with the divisor + **derived** as `factors.height * recipe.max_spatial_downscale()` rather than + hardcoded, which reproduces upstream's 64 and 32 on the two-stage and one-stage + recipes respectively. +- A red-first test entering through `LoadVideoEngine` + `VideoEngine::Generate`, + which is what `vllm_video_generate` itself calls, plus the reachability + mutation. +- `docs/USAGE.md`: the published envelope, and the correction of the frames + claim to what upstream and this tree both actually do. + +**Out.** Named, so none is discovered later as an omission: + +- The res_2s denoising loop and the HQ preset — #921, `## Owed`. +- `TI2VidTwoStagesPipeline` as a distinct recipe row. Our + `DistilledTwoStageRecipe` (`ltx2_pipeline.cpp`, one definition, cited by name + because `:1084-1131` was already mid-function at the base SHA and this row + moves the file again) already carries the + two-phase spatial-upsample shape; the non-distilled variant differs in its + stage-1 schedule and guidance, and is a recipe row rather than a geometry + question. Not bundled. +- Any render, any measurement, any GPU use. `dgx.casa` is running a long render + under `flock` and the box OOM-reboots when its unified pool is exhausted. + +## 3. Design + +### 3.1 The refusal + +One function, mirroring upstream's shape and message content: + +```cpp +void Ltx2AssertResolution(int64_t height, int64_t width, int64_t divisor); +``` + +Called from the geometry block in `ltx2_video.cpp`, after `height` and `width` +resolve and before anything consumes them — which is where upstream calls it, +at the top of `__call__` before any work is paid for. + +**The divisor is derived, not restated.** `recipe.max_spatial_downscale()` +already reports the worst `spatial_downscale` over a recipe's phases: 2 for +`distilled_two_stage`, 1 for `one_stage` and `dmd2`. Multiplied by +`factors.height` (32) it is 64 and 32 — upstream's two numbers, reached by +upstream's reasoning rather than spelled as literals. + +**It reproduces upstream's numbers; it does not generalise past them, and this +row does not claim it does.** The first pass wrote that the derivation "stays +correct for a recipe whose phases downscale by more", and that is stronger than +the code. The quantity a request must survive is the **least common multiple** of +the phase downscales, not the maximum. The two agree on every shipped recipe, +whose downscales are drawn from {1, 2}. They part on a recipe with phases at 2 +and 3: `max` gives a divisor of 96, a 96-wide request passes, and the +downscale-2 phase then floors 48 onto one latent cell — the exact defect this +guard exists to stop. + +**Not implemented, and the reason is reachability rather than effort.** No +shipped recipe has a non-power-of-two spatial downscale, so an lcm form would +change no behaviour any production entry point can reach, and no test entering +through `LoadVideoEngine` could gate it. Landing it would put an unreachable +branch in the tree and a class-level test beside it, which +[`.agents/reachability.md`](../reachability.md) names as the failure to avoid. +Recorded here and in the header comment on `Ltx2AssertResolution` as a stated +limit; it becomes live work the day a recipe with such a phase is added, and the +recipe row that adds one owns it. + +**One divisor covers both axes**, as upstream has it (`divisor = 64 if +is_two_stage else 32`). That is the mirror and not a simplification: upstream's +VIDEO_SCALE_FACTORS is `(8, 32, 32)`, so its single spatial divisor already +covers both. The plan here first said the check would use each factor against its +own axis; that would have been a divergence dressed as future-proofing. What +landed instead asserts `factors.height == factors.width` at the call site and +fails by name if a VAE ever breaks it, so the assumption is checked rather than +carried silently — a VAE with differing axes would otherwise have its width +measured against the height factor and no test would see it. + +**The message is part of the contract, and two halves of it were unheld.** A +refusal that a public document advertises is a promise, so each clause needs a +needle a test can hold and a value a caller can act on. + +*The axis phrase carries its own verb.* The first shape was `"; the " + bad + +" is not"` with `bad` a bare noun. Every refusal this function emits also carries +the literal `" (width x height) "` label, so `msg.find("width")` and +`msg.find("height")` are satisfied by that constant regardless of which axis was +named — a mutation swapping the two names stayed green, and `docs/USAGE.md` +published "the refusal names the offending axis" as a contract no test held. The +phrase now comes out whole: `"the width is not"`, `"the height is not"`, or +`"the width and height are not"`. That third branch is grammatical rather than a +noun spliced into a fixed tail, and it is the branch a caller who passes a square +off-grid size reaches. + +*The suggested size has to be legal.* `(width / divisor) * divisor` is **0** for +any axis below the divisor, so a two-stage width-32 request was told "Nearest +legal size at or below the request: 0x64" — and 0 is refused by the lower bound +in the phase loop a few dozen lines later. That is one illegal size handed out in +place of another, which §Outcome's own standard rejects: a suggestion that is not +itself legal is worse than none. When either floored axis is 0 the message now +says no legal size at or below the request exists, and names the smallest legal +size, which is the divisor on both axes. + +### 3.2 Frames: the doc moves, the code does not + +Upstream floors an explicit frame count and validates it nowhere (§1.1). Our code +floors identically. Mirroring means **not** adding a refusal upstream does not +have, and the honest repair is to `docs/USAGE.md`, which currently promises +enforcement of `(frames - 1) % 8 == 0`. + +This is deliberately the opposite decision from width/height in the same change, +and the asymmetry is upstream's, not ours: `assert_resolution` exists and covers +two of the three axes. Recording the reason here because a later reader will +otherwise read the asymmetry as an oversight. + +The rounding stays observable: `result.frame_count`, `result.width` and +`result.height` report what was rendered, so a caller who checks can still see a +floored request. + +## 4. The ceiling is measured, and it is not attributed + +For the envelope to be a contract it has to say what runs, not only what is +arithmetically legal. Existing evidence in this tree, none of it produced by this +row: + +- **320x192 / 25f completes** on GB10 through both distilled phases + (`docs/USAGE.md`, `docs/BENCHMARKS.md`). +- **448x256 / 25f does not.** It finishes its denoise and then loses about 59 GB + in 24 seconds inside the decode. +- `.agents/specs/ltx25-tiled-decode.md` `## Outcome`: the decode's own heap peak + is 361.72 MiB, which is ~170x too small to be that 60 GiB. That spec states + plainly that the 60 GiB is **NOT attributed**. +- The same spec: `Ltx2ConvVideoDecode` at 448x256/25f took 2681 s, single-threaded, + at 0% GPU. + +`memory_efficient_decode.py` upstream is deliberately unported and is the obvious +candidate for the missing 60 GiB. Attributing it, and the reference decoder's +single-threaded throughput, are two measurement rows that need the GPU. Neither +is this row, and the envelope §6 publishes says so rather than implying the +arithmetic limit is the practical one. + +Set against upstream's own defaults — 1024x1536 and 1088x1920 at 121 frames — a +320x192 practical ceiling is the story, and the envelope states both numbers next +to each other rather than only the legal one. + +## 5. Tests + +Red-first, entering through the production entry point per +[`.agents/reachability.md`](../reachability.md). + +**Which entry point, stated exactly**, because the first pass said +"`vllm_video_generate`" and that is not what the file does. +`tests/vllm/multimodal/test_ltx2_video.cpp:19` includes +`vllm/multimodal/ltx2_video.h`, not `vllm.h`, and every subcase enters at +`LoadVideoEngine` + `engine->Generate`. That is the production entry: the C ABI's +`vllm_video_generate` is a marshalling shell over the same +`VideoEngine::Generate` (`src/capi/vllm_c.cpp:1646` — `engine->engine->Generate(gen)`), +and `tests/capi/test_capi.cpp` gates the ABI hop itself. So reachability holds and +the sentence was wrong, not the test. Nothing constructs `Ltx2AssertResolution`'s +arguments by hand; the divisor a subcase exercises is the one the loaded recipe +produced. + +1. **`test_ltx2_video`** on the distilled two-stage recipe: a request whose width + is not a multiple of 64 is refused, and the message names the divisor and the + offending value. Red before the check exists, because today it renders a + floored clip and returns success. +2. The one-stage divisor of 32 on the same entry point, so the derivation is + gated on both arms and not only on the arm that ships by default. +3. A multiple-of-64 request still resolves, so the refusal is not a blanket one. +4. **Reachability mutation**: delete the `Ltx2AssertResolution` call site in a + scratch copy and rerun the focused gate. A green gate would mean the test + measures the function rather than the capability. +5. **The message's axis phrase, per axis.** A needle of `msg.find("width")` is a + tautology: the message carries the literal `" (width x height) "` label in + every refusal, so both axis words are present whichever axis the guard blamed. + A mutation swapping the two names is green against such needles. Each subcase + therefore asserts the phrase — `"the width is not"` or `"the height is not"` — + and asserts the *other* phrase absent, and a both-axes subcase (80x80) covers + the `"the width and height are not"` branch no other subcase executes. +6. **The suggested size is legal.** `(width / divisor) * divisor` is 0 for any + axis below the divisor, and 0 is refused by the lower bound in the phase loop, + so the suggestion handed one illegal size out in place of another. Sub-divisor + subcases on both recipes assert the "no legal size at or below the request" + wording, the smallest legal size (64x64 and 32x32 respectively, so the value + is proven to follow the derived divisor), and that no `x0` appears. + +**No upstream test is ported, because there is none to port.** `Lightricks/LTX-2` +at `fd4ded7f` contains **zero** `test_*.py` files anywhere in the repository — +`find /home/mudler/_git/LTX-2 -name 'test_*.py'` returns nothing. Recorded as a +fact rather than as a silent omission of the standing "port the upstream tests in +the same change" obligation. + +What replaces that obligation, at the same bar: the tests are written **against +upstream anchors** rather than ported from a suite. Only the provenance changes. +Each still fails for the intended reason before the change, still enters through +`vllm_video_generate` rather than constructing the type, and each asserted +behaviour still names the upstream `file:line` that justifies it — the divisors +and the raise from `helpers.py:540-551`, the halved stage-1 geometry from +`ti2vid_two_stages.py:226-228`, the spatial factor from `ltx_core/types.py:31-33`, +and the frames non-check from `blocks.py:908-928` with `types.py:113`. + +Two of the fixture sizes are **measured rather than reasoned**: width 80 rendering +64x64 and one-stage width 100 rendering 96x64 were observed on this fixture before +the guard existed, which is why the case uses those and not the obvious 96 that +takes a different path entirely. + +## 6. Records + +- `docs/USAGE.md` — the envelope, and the frames correction. Keyed record; + scoped edit reapplied by key with unrelated keys proven byte-identical. +- `docs/FEATURES.md` — **one row appended** to the LTX-2.5 gap table, and nothing + else touched. The row was not in the original plan: this row's first pass + declared FEATURES.md out of scope on the grounds that a refusal changes no + feature surface, and `check-doc-checkpoint` disagreed and was right. A size that + used to render and now refuses is exactly a change in what the project + supports, and the file already carries the LTX-2.5 refusal table that is its + home. Recorded rather than quietly done, because the plan said otherwise. + + The **"Temporal x2 ups gated, UNDRIVEN" cell is untouched**, byte for byte. + This row does not drive that arm and does not change that fact. The file is a + known lock (#595), so the edit is one appended row rather than a rewrite, to + keep the conflict surface with the sibling rows editing it at a single line. +- `.agents/issue-index.md` — append-only, two rows appended, zero removed. +- No lifecycle change, so `docs/STATUS.md`, `docs/BENCHMARKS.md` and `## Now` + are untouched. + +## Owed + +- [#921](https://github.com/mudler/vllm.cpp/issues/921) — the res_2s denoising + loop (`samplers.py:206-447`): the `phi` / `get_res2s_coefficients` exponential + integrator (`res2s.py:4-62`), the second transformer evaluation per step at + `sub_sigma = sqrt(sigma * sigma_next)` (`samplers.py:315`, `:380-386`), the bong + anchor refinement (`samplers.py:357-364`), and the `Ltx2StepperKind` enumerator + to select any of it. Until it lands, + `TI2VidTwoStagesHQPipeline` cannot be served, and serving `LTX_2_3_HQ_PARAMS` + on the Euler loop would render a plausible clip that is quietly not HQ at + roughly half the model evaluations the preset was tuned for. No HQ recipe row + is added by this row, so nothing can select it and nothing lands dead. +- `TI2VidTwoStagesPipeline` as a recipe row — stage 1 on the scheduler-derived + schedule under full CFG, stage 2 on `STAGE_2_DISTILLED_SIGMAS` with guidance + off (`ti2vid_two_stages.py:243-308`). Distinct from the distilled two-stage + recipe that ships. Covered by #644 row scope; not separately filed, because + #644 already owns "close every refused arm". +- Attribution of the 60 GiB decode loss and the single-threaded decode + throughput (§4). Both need the GPU; both are measurement rows. +- **The lcm form of the divisor** (§3.1). `max_spatial_downscale()` is the + maximum where the correct quantity is the least common multiple of the phase + downscales. The two agree on every shipped recipe and part on a recipe with + phases at 2 and 3. Not filed as an issue and not implemented, because no + production entry point can reach the difference today: the recipe row that adds + a phase with a non-power-of-two spatial downscale owns it, and this limit is + stated in the header comment on `Ltx2AssertResolution` so that row finds it. + +## Stop conditions + +- Any need for the GPU. Report and stop. +- If the derived divisor disagrees with upstream's 64/32 on any shipped recipe, + stop and report rather than special-casing a recipe. + +## Outcome + +**The geometry half landed; the sampler half is owed and named.** + +What was measured, and what it changed. The defect was expected to be a single +silent floor and is two failure modes, split by whether a request's floor is +consistent across the phases (§1.2). Width 96 on the two-stage arm — the obvious +test value — takes the *other* path and never floors at all, so a case written +from reasoning rather than measurement would have gated the wrong thing and still +gone green. The fixture sizes are the measured ones for that reason. + +What was rejected. A frames refusal, because upstream floors an explicit count +identically and validates it nowhere; adding one would have been a divergence +wearing the costume of a fix. Per-axis divisors, for the same reason — upstream +has one divisor, and the first draft's "future-proofing" was a divergence too; +what landed asserts the equality the single divisor assumes. And a `two_stage_hq` +recipe row, which would have been selectable by nothing, because the sampler that +makes it HQ is not ported (#921). + +Why the divisor is a parameter rather than a constant: upstream's 64 and 32 are +derived quantities spelled as literals, and spelling them as literals here would +restate the answer instead of the reason. The one mutation that survived the +first pass (§5) was against the message's *suggested* size, not the check — worth +recording, because it is the half of a refusal that gets written once and never +tested, and a suggestion that is not itself legal is worse than none. + +**What the fresh review changed, recorded rather than quietly fixed.** The guard +itself survived unchanged in position, derivation and reachability. Everything +the review moved was around it, and the pattern is worth naming: *a claim is not +evidence, and a needle read out of the file it validates is not a test.* + +- **The axis-naming assertions were tautologies.** Four subcases asserted + `msg.find("width")` / `msg.find("height")` against a message that carries + `" (width x height) "` in every refusal it emits. Swapping the two names in the + guard was **green** across the whole file. The needles are now phrases, each + subcase asserts the other axis absent, and a both-axes subcase covers a branch + no test executed. `docs/USAGE.md` had already published "the refusal names the + offending axis" as a contract, so this was a published promise nothing held. +- **The suggested size could be illegal.** §3.1. Measured at the unmutated head, + not reasoned: a two-stage width-32 request was told to render at `0x64`. +- **Three counts and one premise were wrong, and none of them came from the + code.** "Ten call sites" is nine invocations; "every pipeline's `__call__`" is + nine of thirteen; `snap_frames_to_grid` has three callers and not one. Each was + a number or a sentence carried forward rather than counted, which is the + failure mode `.agents/verification.md` warns about — the count nobody ran. + §1.1 and §1.2 now show the counting, so the next reader can check it. +- **Every repo-local anchor in the appended issue-index rows is now SHA-pinned.** + They were correct at the base SHA and wrong at the landing tree, because the + row inserts lines above them and then `origin/main` moved again for #935. The + index is append-only: a row that lands wrong cannot be corrected (#911). +- **A bare `:NN` continuation is only as good as its antecedent.** The first + repair pass of this document introduced four of them with no named file in + scope at all, and three more whose nearest named file was ambiguous — upstream + has several `helpers.py` and `blocks.py`. Found by sweeping the anchors this + branch *adds*, resolving each against the tree it claims. Reading them out of + the cited file would have reported all of them fresh. + +State: complete for its scope. No lifecycle row moves, so `docs/STATUS.md`, +`docs/BENCHMARKS.md` and `.agents/NOW.md` are untouched by design. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index c71a9abde..0d688c2a1 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -334,6 +334,7 @@ CPU elementwise GEMM (f32/f16/bf16) runs AVX2 and AVX-512 tiers on x86 where the | Muse Glimmer 30B (Meta) | Text gated at **reduced depth 4/52** only; vision wired but never reference-checked | [spec](../.agents/specs/muse-glimmer.md) / [#268](https://github.com/mudler/vllm.cpp/issues/268). Full depth, multi-step decode, image/video, server path and parser scoping open. vLLM speed OPEN GAP; llama.cpp bar #333 | | LTX-2.5 AUTO duration (the duration head) | Brick ported, never constructed | `duration_head_path` is REFUSED by name rather than accepted-and-ignored ([#611](https://github.com/mudler/vllm.cpp/issues/611)); supplying a head cannot load one. Give `num_frames` or `duration` | | LTX-2.5 arms a request CAN reach | Refused by name at the call site | The spatiotemporal latent upsampler (both flags set). Supplying that checkpoint names that arm, not the temporal one. The temporal-only x2 arm is ported, not refused | +| LTX-2.5 resolution | Off-grid sizes refused, naming the offending axis and a size you can actually pass; frames still round | `--width`/`--height` must divide 64 (two-stage) or 32 (one-stage), from the VAE factor times the phase downscale ([#919](https://github.com/mudler/vllm.cpp/issues/919)). `--frames` rounds to `8k + 1`. No size cap | | LTX-2.5 arms nothing can request | Declared, not requestable | LoRA fusion, `int8-convrot` (ComfyUI-only), single-node multi-GPU, `BetaScheduler` (upstream selects no scheduler either). No flag or extra asks for these. `multishot` was RETIRED: no such entry point exists upstream | | Multi-GPU execution | Hardware-blocked | TP proven equal to tp=1 on CPU; no 2-GPU box to run it | | LoRA end to end | CPU brick landed | Unwired standalone; not usable through the server | diff --git a/docs/USAGE.md b/docs/USAGE.md index 54e44c857..9d3e5bc2c 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -656,10 +656,56 @@ both numbers in the message — a resampled-wrong, upmixed or silence-padded tak renders a finished clip conditioned on audio nobody supplied. This needs an audio VAE that carries encoder weights; a decoder-only one refuses by name. -`--frames` must satisfy `(frames - 1) % 8 == 0` and width/height must divide by -64 (32 for the VAE, twice that because the distilled recipe's first phase runs at -half resolution). Omitting all three renders the recipe default, which is -1024x1536 at 121 frames and is a much larger request than it looks. +#### The supported resolution envelope + +**`--width` and `--height` are enforced, and an unsupported value is refused by +name.** Both must be multiples of the VAE's spatial factor (32) times the worst +downscale the recipe's phases apply — so **64 on the distilled two-stage recipe**, +whose first phase runs at half resolution, and **32 on a one-stage recipe**. Those +are upstream's own two numbers (`assert_resolution`, +`ltx-pipelines utils/helpers.py:540-551`), reached by upstream's derivation rather +than hardcoded, so a recipe that downscaled further would tighten the divisor with +it. The refusal names the offending axis — width, height, or both — the divisor, +and a size you can actually pass: the nearest legal one at or below the request, +or, when an axis is smaller than the divisor and no such size exists, the +smallest legal size there is. + +Until 2026-08-15 nothing enforced this and the engine floored instead: a +two-stage request of width 80 rendered 64 and returned success, and a one-stage +request of width 100 rendered 96 ([#919](https://github.com/mudler/vllm.cpp/issues/919)). + +**`--frames` is NOT enforced, and it rounds.** A frame count is floored onto the +VAE's temporal grid, `(frames - 1) / 8 * 8 + 1`, so 100 frames renders 97. This +mirrors upstream, which floors an explicit `num_frames` identically +(`ltx_core/types.py:113`) and validates it nowhere: its `snap_frames_to_grid` +helper is called from the auto-duration path and from the dubbing pipeline, and +that pipeline takes no frame count at all — it snaps one read from a reference +video's container. No frame count a caller supplies is snapped or checked, in +either project. Pass a value of the form `8k + 1` to get exactly what you asked +for. The rounding is observable either way: `result.frame_count`, `result.width` +and `result.height` report what was actually rendered, not what was requested. + +Omitting all three renders the recipe default, which is 1024x1536 at 121 frames +and is a much larger request than it looks. + +**What is legal is not what fits.** The first two rows below are a property of +this port and are enforced. The rest are scale markers, and the last two are +measurements of one box rather than limits of the code: + +| | Value | +|---|---| +| Legal sizes | any multiple of 64 (two-stage) or 32 (one-stage), on both axes | +| Legal frame counts | any; non-`8k + 1` values floor onto the temporal grid | +| Upstream's default output | 1024x1536 at 121 frames (`utils/constants.py:42-76`) | +| Upstream's HQ preset output | 1088x1920 at 121 frames (`utils/constants.py:95-98`) | +| **Measured to complete on one GB10** | **320x192 at 25 frames** | +| Measured NOT to complete | 448x256 at 25 frames — the denoise finishes, then the decode loses about 59 GB in 24 s | + +That gap between the legal envelope and the measured one is a decode problem, not +a resolution cap: there is no maximum-size check anywhere in this path, and the +60 GB is **not attributed** — the decode's own heap peak at that size is 361.72 +MiB, some 170x too small to account for it. See the note below on what bounds a +render, and `.agents/specs/ltx25-tiled-decode.md`. `--upsampler` is what the distilled recipe's second phase needs. Without it that phase refuses rather than skipping: its three-step refinement is what makes the diff --git a/include/vllm/model_executor/models/ltx2_pipeline.h b/include/vllm/model_executor/models/ltx2_pipeline.h index 3e87cfeff..07111a3b8 100644 --- a/include/vllm/model_executor/models/ltx2_pipeline.h +++ b/include/vllm/model_executor/models/ltx2_pipeline.h @@ -565,6 +565,44 @@ struct Ltx2PipelineRecipe { int64_t max_spatial_downscale() const; }; +// `assert_resolution` (ltx-pipelines utils/helpers.py:540-551). Upstream calls it +// at the top of a pipeline's `__call__`, before any work is paid for — NINE +// invocations, counted at the pin, among them ti2vid_two_stages.py:184 and +// ti2vid_two_stages_hq.py:199 (both `is_two_stage=True`) against +// ti2vid_one_stage.py:156 (`False`). Nine, and not the twenty-one lines a grep +// for the name returns: those are 9 invocations + 1 definition + 10 imports + 1 +// `__all__` string. Nor is it every pipeline: 13 pipeline `__call__`s take a +// height and a width, and the four that do NOT call the guard are +// distilled_mgpu.py:143, ti2vid_two_stages_mgpu.py:163, +// ti2vid_two_stages_hq_mgpu.py:164 and hdr_ic_lora.py:352. +// +// Upstream spells the divisor as a literal 64 or 32 chosen by a bool. That pair +// is not two constants: it is the VAE spatial factor (32, +// ltx_core/types.py:31-33) times the worst spatial downscale any phase applies. +// A two-stage pipeline runs stage 1 at `width // 2` +// (ti2vid_two_stages.py:226-228), so the request must survive being halved and +// still divide the grid — hence 32 * 2. Taking the divisor as a parameter lets +// the caller derive it from the recipe it actually holds, which reproduces +// upstream's two numbers on the two shipped arms rather than restating them as +// literals. +// +// It reproduces them; it does not generalise past them, and the limit is stated +// rather than implied. `max_spatial_downscale()` takes the MAXIMUM, and the +// quantity a request must survive is the LEAST COMMON MULTIPLE of the phase +// downscales. The two agree on every shipped recipe, whose downscales are 1 and +// 2, and they part on a recipe with phases at 2 and 3: the max gives 96, a +// 96-wide request passes, and the downscale-2 phase then floors 48 onto one +// latent cell — the very defect this guard exists to stop. No shipped recipe has +// a non-power-of-two downscale, so the lcm form would change no behaviour any +// production entry point can reach and no test entering there could gate it. +// Recorded as a limitation in `.agents/specs/ltx25-resolution-envelope.md` +// instead of implemented unreached. +// +// This is a REFUSAL and not a rounding on purpose. Integer division is what the +// engine did before, and it renders a clip at a size nobody asked for +// (#919) — 80 became 64, successfully, with the wrong size reported back. +void Ltx2AssertResolution(int64_t height, int64_t width, int64_t divisor); + // resolve_ltx_pipeline_recipe (ltx2_recipes.py:170-175). Keyed on the EXACT // (pipeline_kind, model_version) pair and throwing by name on anything else — // never defaulting. The table: diff --git a/src/vllm/model_executor/models/ltx2_pipeline.cpp b/src/vllm/model_executor/models/ltx2_pipeline.cpp index a54f87763..a0d3e4ed9 100644 --- a/src/vllm/model_executor/models/ltx2_pipeline.cpp +++ b/src/vllm/model_executor/models/ltx2_pipeline.cpp @@ -967,6 +967,49 @@ int64_t Ltx2PhaseRecipe::num_inference_steps() const { return sigmas.empty() ? -1 : static_cast(sigmas.size()) - 1; } +void Ltx2AssertResolution(int64_t height, int64_t width, int64_t divisor) { + Require(divisor >= 1, "ltx2 resolution: the divisor must be at least 1, got " + + std::to_string(divisor)); + // Upstream checks both axes against one divisor and names both in one message + // (helpers.py:546-551). Naming the OFFENDING axis as well, because a caller who + // passed two numbers cannot tell from "(80x64) is not divisible by 64" which of + // them to change — and the fix is a different number on each axis. + // + // The axis phrase carries its own verb rather than being a bare noun dropped + // into one template. A shared "; the X is not" tail reads as a constant to a + // test: `msg.find("width")` is satisfied by the "(width x height)" label this + // message always carries, so a needle spelt that way stays green with the two + // names SWAPPED. The needle a test can hold has to be the phrase, not the word. + if (height % divisor != 0 || width % divisor != 0) { + const std::string bad = + (height % divisor != 0) + ? (width % divisor != 0 ? "the width and height are not" : "the height is not") + : "the width is not"; + // A suggestion has to be a size the caller can actually pass. Flooring an axis + // that is BELOW the divisor yields 0, and 0 is refused three lines further on + // by the lower bound, so the old wording sent a width-32 caller from one + // refusal to another. When either axis floors away there is no legal size at + // or below the request at all, and saying so — with the smallest legal size — + // is the honest answer. + const int64_t floor_w = (width / divisor) * divisor; + const int64_t floor_h = (height / divisor) * divisor; + const std::string suggestion = + (floor_w == 0 || floor_h == 0) + ? ("No legal size at or below the request exists: an axis under " + + std::to_string(divisor) + " floors to zero latent cells. Smallest legal size: " + + std::to_string(divisor) + "x" + std::to_string(divisor)) + : ("Nearest legal size at or below the request: " + std::to_string(floor_w) + "x" + + std::to_string(floor_h)); + Refuse("ltx-2.5 video: the requested resolution " + std::to_string(width) + "x" + + std::to_string(height) + " (width x height) is not divisible by " + + std::to_string(divisor) + "; " + bad + ". That divisor is this " + + "recipe's worst phase downscale times the VAE's spatial factor, so both axes " + + "must be multiples of " + std::to_string(divisor) + ". " + suggestion + + " (`assert_resolution`, ltx-pipelines utils/helpers.py:540-551). Rounding it " + + "here would render a clip at a size nobody asked for"); + } +} + int64_t Ltx2PipelineRecipe::max_spatial_downscale() const { int64_t worst = 1; for (const Ltx2PhaseRecipe& phase : phases) { diff --git a/src/vllm/multimodal/ltx2_video.cpp b/src/vllm/multimodal/ltx2_video.cpp index 495973cdc..ce438a27c 100644 --- a/src/vllm/multimodal/ltx2_video.cpp +++ b/src/vllm/multimodal/ltx2_video.cpp @@ -1503,6 +1503,49 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { const Ltx2ScaleFactors factors; // VIDEO_SCALE_FACTORS (types.py:70) — the conv // arm's fixed (8, 32, 32), not derived // (utils/helpers.py:66-72) + + // `assert_resolution` (utils/helpers.py:540-551), at the position upstream + // calls it from: the top of `__call__`, before any work is paid for. The + // divisor is DERIVED — the VAE spatial factor times the worst downscale this + // recipe's phases apply — which is upstream's own 64 for a two-stage recipe and + // 32 for a one-stage one, reached by upstream's reasoning rather than restated + // as two literals. + // + // FRAMES ARE DELIBERATELY NOT CHECKED HERE, and the asymmetry is upstream's. + // `resolve_num_frames` (utils/blocks.py:908-928) returns an explicit count + // verbatim (utils/blocks.py:920-921) and `VideoLatentShape.from_pixel_shape` + // (ltx_core/types.py:113) + // then floors it exactly as the `vshape.frames = (frames - 1) / factors.time + 1` + // line in the phase loop below does. Adding a refusal here would be a divergence + // from the reference, not a mirror of it — so `docs/USAGE.md` carries the + // rounding as documented behaviour instead (#919). + // + // `snap_frames_to_grid` (utils/helpers.py:554-562) does NOT contradict that, + // and the reason is not the one it is easy to give. It has three callers, not + // one: utils/helpers.py:581 inside `seconds_to_clamped_num_frames`, which is the + // auto-duration path, and dubit.py:215 and :396, the second of which is inside + // `DubitPipeline.__call__` three lines after its own `assert_resolution`. So + // "only the auto-duration path snaps" is false. What holds is sharper: + // `DubitPipeline.__call__` takes NO `num_frames` at all (dubit.py:194-210) and + // snaps a count it read from the reference video's container metadata. Counted + // at the pin, it is the only pipeline `__call__` that snaps, and the only one + // with no `num_frames` parameter — every `__call__` that does take one leaves it + // unsnapped. An explicit frame count is floored upstream and here, and validated + // in neither. + // ONE divisor for both axes, as upstream has (`divisor = 64 if is_two_stage + // else 32`). That is a mirror and not a simplification: upstream's + // VIDEO_SCALE_FACTORS is (8, 32, 32), so its single spatial divisor already + // covers both axes. The equality is asserted rather than assumed, because a VAE + // whose axes differed would otherwise have its width checked against the height + // factor and no test would see it. + if (factors.height != factors.width) { + Fail("the VAE's spatial scale factors differ (" + std::to_string(factors.height) + + " high, " + std::to_string(factors.width) + + " wide), so one resolution divisor cannot cover both axes the way " + "`assert_resolution` (utils/helpers.py:540-551) does"); + } + Ltx2AssertResolution(height, width, factors.height * recipe.max_spatial_downscale()); + // `AudioLatentShape.from_video_pixel_shape` (types.py:184-200) and // `VideoLatentShape.from_pixel_shape` (:108-123) defaults. Asserted against the // DiT rather than assumed: the audio latent's channels x mel_bins IS the audio diff --git a/tests/vllm/multimodal/test_ltx2_video.cpp b/tests/vllm/multimodal/test_ltx2_video.cpp index ad2ed0719..aa36dc852 100644 --- a/tests/vllm/multimodal/test_ltx2_video.cpp +++ b/tests/vllm/multimodal/test_ltx2_video.cpp @@ -514,6 +514,252 @@ TEST_CASE("ltx2 video: the second phase upsamples, and refuses when it cannot") } } +// A request whose size does not divide the latent grid used to RENDER at a size +// nobody asked for (#919). `ltx2_video.cpp` integer-divides the request into +// `Ltx2VideoLatentShape` and only ever checked the LOWER bound, so the floor was +// silent and the call returned success. +// +// MEASURED on this fixture before the guard existed, which is why these sizes and +// not the obvious ones: a two-stage request of width 80 rendered 64x64, and a +// one-stage request of width 100 rendered 96x64. Width 96 on the two-stage arm +// does NOT reach that state — stage 1 floors 48 to one latent cell while stage 2 +// needs three — so the upsampler's shape check catches it and reports "the +// upsampled latent is 4x2x2x2 but phase 'refine' needs 4x2x2x3", a true statement +// about latents and no help at all to a caller who passed a width. The defect has +// two faces, a silent floor and an unreadable downstream throw, and one guard at +// the entry point closes both. +// +// Upstream raises instead, at the top of a pipeline's `__call__` and before any +// work is paid for: `assert_resolution` (ltx-pipelines utils/helpers.py:540-551) +// takes a divisor of 64 for a two-stage pipeline and 32 for a one-stage one, +// across NINE invocations including ti2vid_two_stages.py:184 and +// ti2vid_two_stages_hq.py:199. Nine, counted at the pin: a grep for the name +// returns 21 lines, which are 9 invocations + 1 definition + 10 imports + 1 +// `__all__` string. Nor is the guard on every pipeline — 13 pipeline `__call__`s +// take a height and a width, and distilled_mgpu.py:143, +// ti2vid_two_stages_mgpu.py:163, ti2vid_two_stages_hq_mgpu.py:164 and +// hdr_ic_lora.py:352 do not call it. +// +// Those two divisors are NOT two constants. They are the VAE spatial factor (32, +// ltx_core/types.py:31-33) times the worst spatial downscale any phase applies — +// a two-stage pipeline runs stage 1 at `width // 2` (ti2vid_two_stages.py:226-228), +// so the request must survive being halved and still divide the grid. The last two +// subcases gate that DERIVATION rather than the numbers, by driving the same width +// 96 through both recipes and requiring opposite answers. +// +// EVERY AXIS ASSERTION HERE IS A PHRASE AND NOT A WORD, and that is the whole +// point of the spelling. The message carries the literal "(width x height)" label +// in every refusal it ever emits, so `msg.find("width")` and `msg.find("height")` +// are both satisfied by that constant no matter which axis the guard named — a +// mutation swapping the two names in `Ltx2AssertResolution` stayed green against +// needles spelt that way, and a height-80 request would have reported "the width +// is not" with nothing to see it. +TEST_CASE("ltx2 video: a size that does not divide the latent grid is REFUSED, per recipe") { + Workspace ws; + + SUBCASE("a two-stage width that is not a multiple of 64 is refused BY VALUE") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras["upsampler_path"] = ws.paths.upsampler; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/w80"); + gen.width = 80; + try { + (void)engine->Generate(gen); + FAIL("width 80 rendered 64x64 before the guard; it must be refused, not floored"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + // The offending value, the divisor, and the axis. A message that says only + // "bad resolution" leaves the caller to guess which of the two numbers they + // passed is wrong and what a right one would be. + CHECK(msg.find("80") != std::string::npos); + CHECK(msg.find("64") != std::string::npos); + // The PHRASE. Only the width offends here, so the message must say so and + // must not name the height. + CHECK(msg.find("the width is not") != std::string::npos); + CHECK(msg.find("the height is not") == std::string::npos); + // The suggested size, and that it is the NEAREST legal one rather than the + // request echoed back. Without this the arithmetic is unmeasured: a mutation + // replacing `(width / divisor) * divisor` with `width` left all fifteen + // other assertions green, and a wrong suggestion here sends the caller + // straight to another illegal size. + CHECK(msg.find("Nearest legal size at or below the request: 64x64") != std::string::npos); + } + } + + SUBCASE("a two-stage height that is not a multiple of 64 is refused BY VALUE") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras["upsampler_path"] = ws.paths.upsampler; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/h80"); + gen.height = 80; + try { + (void)engine->Generate(gen); + FAIL("height 80 must be refused, not floored to 64"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + CHECK(msg.find("80") != std::string::npos); + // The axis the guard NAMED, not the axis label the message always carries. + // This is the assertion the swap mutation has to move: the width is 64 here + // and legal, so a message that blames it is wrong. + CHECK(msg.find("the height is not") != std::string::npos); + CHECK(msg.find("the width is not") == std::string::npos); + CHECK(msg.find("Nearest legal size at or below the request: 64x64") != std::string::npos); + } + } + + // BOTH axes off the grid. This branch of the axis phrase is executed by no + // other subcase, and it is the one a caller passing a square off-grid size + // reaches — the commonest shape of the mistake. + SUBCASE("a two-stage request with BOTH axes off the grid names both") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras["upsampler_path"] = ws.paths.upsampler; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/wh80"); + gen.width = 80; + gen.height = 80; + try { + (void)engine->Generate(gen); + FAIL("80x80 is off the grid on both axes and must be refused"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + CHECK(msg.find("the width and height are not") != std::string::npos); + CHECK(msg.find("Nearest legal size at or below the request: 64x64") != std::string::npos); + } + } + + // A SUB-DIVISOR axis, where the suggestion the refusal makes is the thing under + // test. `(width / divisor) * divisor` is 0 for any width below the divisor, and + // 0 is not a legal size: a caller who followed the old "Nearest legal size at or + // below the request: 0x64" landed on the LOWER-bound refusal in the phase loop, + // one illegal size handed out in place of another. No subcase reached this + // branch, because every measured size in this case is above its divisor. + SUBCASE("a two-stage width BELOW the divisor is not told to render at zero") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras["upsampler_path"] = ws.paths.upsampler; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/w32"); + gen.width = 32; + try { + (void)engine->Generate(gen); + FAIL("width 32 is below the two-stage divisor of 64 and must be refused"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + CHECK(msg.find("the width is not") != std::string::npos); + // What it must NOT say. `0x64` is the old suggestion, and `x0` catches the + // mirrored case on the height axis. + CHECK(msg.find("0x64") == std::string::npos); + CHECK(msg.find("x0") == std::string::npos); + // What it must say instead: that no legal size at or below the request + // exists, and what the smallest legal one is. + CHECK(msg.find("No legal size at or below the request exists") != std::string::npos); + CHECK(msg.find("Smallest legal size: 64x64") != std::string::npos); + } + } + + // The same branch on the other recipe, so the smallest legal size is proven to + // follow the DERIVED divisor rather than being a second hardcoded 64. + SUBCASE("a one-stage width BELOW the divisor names 32 as the smallest legal size") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras[vllm::multimodal::kLtx2PipelineKindExtra] = "one_stage"; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/os16"); + gen.width = 16; + try { + (void)engine->Generate(gen); + FAIL("width 16 is below the one-stage divisor of 32 and must be refused"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + CHECK(msg.find("the width is not") != std::string::npos); + CHECK(msg.find("No legal size at or below the request exists") != std::string::npos); + CHECK(msg.find("Smallest legal size: 32x32") != std::string::npos); + CHECK(msg.find("x0") == std::string::npos); + } + } + + // The other face of the same defect: 96 reaches the upsampler's shape check + // today. After the guard it is refused at the entry point, by the value the + // caller actually passed. + SUBCASE("a two-stage width of 96 is refused BY VALUE, not by latent shape") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras["upsampler_path"] = ws.paths.upsampler; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/w96"); + gen.width = 96; + try { + (void)engine->Generate(gen); + FAIL("96 is not a multiple of 64 and must be refused at the entry point"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + CHECK(msg.find("96") != std::string::npos); + CHECK(msg.find("the width is not") != std::string::npos); + CHECK(msg.find("the height is not") == std::string::npos); + CHECK(msg.find("upsampled latent") == std::string::npos); + } + } + + SUBCASE("a one-stage width that is not a multiple of 32 is refused BY VALUE") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras[vllm::multimodal::kLtx2PipelineKindExtra] = "one_stage"; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/os100"); + gen.width = 100; + try { + (void)engine->Generate(gen); + FAIL("one-stage width 100 rendered 96x64 before the guard; it must be refused"); + } catch (const std::exception& e) { + const std::string msg = e.what(); + INFO(msg); + CHECK(msg.find("100") != std::string::npos); + CHECK(msg.find("32") != std::string::npos); + CHECK(msg.find("the width is not") != std::string::npos); + CHECK(msg.find("the height is not") == std::string::npos); + // 96, not 64: the nearest legal size follows the recipe's own divisor. + CHECK(msg.find("Nearest legal size at or below the request: 96x64") != std::string::npos); + } + } + + // The guard is not a blanket one. Without this, every subcase above is + // satisfied by a check that refuses everything. + SUBCASE("a multiple of 64 still renders on the two-stage recipe") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras["upsampler_path"] = ws.paths.upsampler; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + const vllm::multimodal::VideoResult result = engine->Generate(FixtureGen(ws.root + "/ok64")); + CHECK(result.width == 64); + CHECK(result.height == 64); + } + + // The SAME width 96 the two-stage arm refuses, on a recipe whose only phase + // runs at the requested size. 96 = 3 * 32, so it divides that grid and must be + // served. A hardcoded 64 would refuse it here too: this is the assertion that + // separates the derivation from the constant. + SUBCASE("96 is a legal ONE-STAGE size, because that divisor is 32") { + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + mp.extras[vllm::multimodal::kLtx2PipelineKindExtra] = "one_stage"; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/one_stage_96"); + gen.width = 96; + const vllm::multimodal::VideoResult result = engine->Generate(gen); + CHECK(result.width == 96); + CHECK(result.height == 64); + } +} + // ─── the refusals, each of which would otherwise RENDER ───────────────────── // PHASE L8 CHANGED WHAT THIS CASE ASSERTS, and the change is the phase.