port(ltx-2.5): L10 — a text prompt drives the 12B tower, and the oracle L3 said did not exist (#435) - #554
Merged
Conversation
…le L3 said did not exist (#435) LTX-2.5 advertises text-to-video and could not be typed at. `has_encoder()` was false and a prompt-carrying request was refused by name. This makes the prompt path RUN: a real string tokenizes with the tokenizer the checkpoint ships as a TENSOR, the torchao-NVFP4 Gemma-4 tower is materialized and forwarded, all 49 hidden states feed the existing aggregation, and both caption projections emit the two conditioning streams. One commit because the doc-checkpoint gate is per-commit and every part owes the same two documents; the four blocks below are how a reviewer should read it. ──────────────────────────────────────────────────────────────────────────────── 1. THE ORACLE L3 RECORDED AS IMPOSSIBLE WAS A `transformers` VERSION Phase L3 recorded that the tower could not be gated: no `gemma4_unified` in `CONFIG_MAPPING`, so it could not be built at reduced dims and there was nothing to compare against. That was never a property of the tower. MEASURED both ways on this box: /usr/bin/python3 transformers 5.3.0 -> KeyError 'gemma4_unified' /home/mudler/recon-cpu/venv transformers 5.12.1 -> builds and RUNS the tower So `Gemma4Model::ForwardHiddenStates` is now held to a RUNNING upstream instead of to invariants derived from its own output. gemma4.h has said "grounded + compiles" since it landed; compiling is not running. THE TOLERANCE IS A MEASUREMENT. Our stream carries bf16 and widens only on the way out; bf16 is also upstream's own resolved model dtype (base_encoder.py:41). So "is a difference a defect?" has a measurable answer: how far does UPSTREAM's answer move when the SAME upstream code runs at bf16 instead of f32? The generator measures exactly that, per state, and emits it as the bound. Nobody picked it, and it cannot be loosened to rescue a failure — loosening it means regenerating it, which means the oracle moved. state ours vs bf16 oracle the floor ratio 0 0 0.000923 0.00x 1 0.03125 0.072665 0.43x 3 0.191406 0.305009 0.63x 6 0.34375 0.559325 0.61x 9 0.46875 0.683670 0.69x 11 0.650391 1.098824 0.59x 12 0.149658 0.210947 0.71x Worst 0.71x over all 13 states. We are closer to upstream-in-bf16 than upstream-in-bf16 is to upstream-in-f32, at every layer; state 0 is bit-identical. The f32 arm is gated too, at the triangle bound, because a bf16 store is exactly what absorbs a reduction-order defect. THE FIXTURE DISCRIMINATES (§7.0(c)). The shipped 12B is not one repeated layer: (sliding x 5, full) eight times, and the two kinds differ — 16 q heads x 256 with 8 kv heads and a `v_proj`, against 16 x 512 with ONE kv head and no `v_proj` at all (`attention_k_eq_v` aliases V onto K), plus two rope types at two thetas and partial rotary on the full arm only. The reduced arch keeps every one and repeats the pattern twice, so a full layer is neither first nor last and a sliding layer FOLLOWS a full one. MUTATION-PROVEN: dropping `layer_scalar` -> 34 assertions RED; capturing the RAW last layer instead of the final-normed one -> 3 RED. Tree restored byte-for-byte (md5 040b97bf6b753f04be1855eaefc76ea1) and green re-confirmed after each. THE CONFIG IS NOT GUESSED. The shipped vonkaiser build has NO `__metadata__`, so its Gemma config cannot come from the file — which is also why upstream's own `GemmaAssets.from_single_file` raises on it. The committed config is the `__metadata__["gemma_config"]` of the OFFICIAL bf16 text encoder, read with an 84 KB HTTP RANGE request over the safetensors header: no large download, and the authoritative source rather than a reconstruction. It independently confirms §1.4's corrected width — `video_aggregate_embed.weight` is BF16 [4096, 188160] there, unpacked, where the NVFP4 build stores [4096, 94080]. `ParseHfConfig(json, source)` exists for that: a config does not always arrive as a sibling config.json, and writing one to a temp file to read it back would put the temp directory on a model path. ──────────────────────────────────────────────────────────────────────────────── 2. THE PROMPT PATH, AND A TOKENIZER THAT SHIPS AS A TENSOR LTX-2.5's text encoder embeds its tokenizer as `tokenizer_json` U8 [32169626] inside the one safetensors file (gemma_assets.py:34-36). Every loader in this tree assumed a sibling `tokenizer.json`, so none could read this checkpoint; hence `Tokenizer::FromHfJsonBytes`. GATE 1 — the WRAPPER, inline fixture, no asset. Strip; BOS prepended CONDITIONALLY (prepending unconditionally doubles it and drops the last token); LEFT padding so the valid run is the TAIL; truncate-then-prepend-then-re-truncate so a maximal prompt loses its LAST token rather than its BOS; an empty prompt still carrying exactly one BOS; and a refusal with no bos id, where upstream also raises (tokenizer.py:34-36). GATE 2 — REAL prompts, the REAL 262144-entry vocab, TOKEN-EXACT. Four prompts x 1024 positions, ids and mask, vs HuggingFace `tokenizers` 0.22.2: prompt 0 (10 valid tokens): first mismatching index = -1 (-1 = EXACT) prompt 1 (11 valid tokens): -1 prompt 2 ( 1 valid token ): -1 prompt 3 (43 valid tokens): -1 No tolerance appears in that gate. The prompts discriminate: one with leading and trailing whitespace, one EMPTY, one 43-token cinematic prompt with punctuation and digits. THE DISAGREEMENT, followed to the model author and recorded. `ltx_core` prepends BOS explicitly and says why — "Gemma 3 already emits it via post_processor; Gemma 4 does not, so we prepend" (tokenizer.py:12-15). diffusers instead passes `add_special_tokens=True` (pipeline_ltx2.py:339). MEASURED on the shipped tokenizer: its post_processor is a TemplateProcessing with ZERO special_tokens, so it adds nothing and diffusers would produce every prompt without a BOS. That measurement is emitted into the goldens header, so the reason is attached to the artifact. ──────────────────────────────────────────────────────────────────────────────── 3. THE TOWER ITSELF, CLOSING L6's RECORDED DEBT L6 recorded verbatim: "the torchao arm of the Gemma TOWER itself (`Gemma4Weights`) is not wired ... L6 loads the two caption projections, the asset pack and the geometry, and VALIDATES every tower module without materializing it." `Ltx2LoadGemmaTowerFromSafetensors` closes it, reusing L6's own `Ltx2DequantTorchaoNvfp4ToBf16` unchanged. WHY NOT `gemma4_weights.cpp`. The LTX file ships FLAT names (`model.layers.{i}.self_attn.q_proj.weight`); that loader reads the multimodal-wrapper form `model.language_model.layers.{i}....`. And every U8 width is HALF its logical one, so each layer's geometry is resolved from the CONFIG and the stored shapes are checked against it, never the reverse — a loader that trusted stored widths would build a tower of exactly half the right width whose matmuls all still conform among themselves. REFUSALS, each naming a specific wrong model: a PLE config against a checkpoint with no PLE tensors; `num_kv_shared_layers` != 0, where a shared layer reads a cache nobody wrote; a `v_proj` present or absent against what `attention_k_eq_v` declares, which is 16 kv heads of difference that every shape check passes; and a module in neither the BF16 nor the torchao-NVFP4 form. THE COST THE PORT DOES NOT PAY. Upstream pads every prompt to 1024 and runs all 1024 rows through a 12B tower; this runs the prompt's OWN length at the original absolute positions. Both halves of that equivalence are MEASURED, not argued: upstream's own padded-vs-short f32 spread is 3.2e-05 over a max|value| of 14.35, and our short run reproduces the padded run's valid rows at 0.5x the dtype bound. Writing that gate the obvious wrong way is kept in the test rather than quietly fixed: declaring seq_len = P + T while writing only T keys has attention read P slots of zeroes and call them keys — max|diff| 17.97 against max|value| 14.35. MEASURED END TO END on the real checkpoint (opt-in; ~33 GB host, ~26 min CPU): 329 torchao-NVFP4 modules dequantized; 48 layers, 8 of them full-attention at head_dim 512 / 1 kv head / no v_proj, 40 sliding at 256 / 8 kv heads prompt "a red fox running through deep snow at sunrise" -> 10 tokens -> video [1024, 4096] max|v| = 34.0715 mean|v| = 0.0401384 -> audio [1024, 2048] max|v| = 27.1555 a DIFFERENT prompt differs by max|diff| = 33.7318 6,292,805 assertions, 0 failed VALUES, not `isfinite`: a conditioning tensor of zeros would satisfy every shape and finiteness check and render an unconditioned clip, and a tower that ignored its input entirely would pass everything except the two-prompt arm. ──────────────────────────────────────────────────────────────────────────────── 4. WHY `has_encoder()` IS STILL FALSE Upstream does not hand the caption projections to cross-attention. Each stream goes through an `Embeddings1DConnector` first (embeddings_processor.py:70-117). Its MATH is ported — `Ltx2ConnectorForward` — but its WEIGHTS are not loaded: they ship inside the DiT file as `video_embeddings_connector.*` / `audio_embeddings_connector.*` and remain among the modules `Ltx2LoadDitFromSafetensors` REFUSES (ltx2_loader.h:96-99). MEASURED in the shipped FP8 DiT header: 372 such tensors, `learnable_registers [128, 2048]` on the audio arm, and `attn2.to_k.weight [4096, 4096]` confirming the video stream's 4096 is exactly the width the projections emit. So the conditioning this change computes has nowhere to go, and a `true` here would promise a render that cannot complete. The refusal therefore MOVES rather than lifts: `encoder_path` is still refused, but the message now names the connector weights instead of the tower, because the old message — "the Gemma-4 TOWER itself is owed" — is no longer true, and a stale refusal is worse than a blunt one. Flipping the flag is one change away, in files this row does not own. ──────────────────────────────────────────────────────────────────────────────── BASELINES, all matching, none moved test_ltx2 29/1615 test_ltx2_vae 16/1816 test_ltx2_pipeline 35/2358 test_ltx2_loader 20/2363 test_ltx2_video 17/170 test_ops_attention_cross 9/32 test_minimax_h3 79/57395 test_minimax_h3_video_fold 6/137 test_video_engine 11/254 test_capi 55/505 test_gemma4_honesty 2/6 test_gemma4_rocm_fp8_seams 2/10 test_gemma3_forward 3/503 test_tool_parser_gemma4 72/161 test_gemma4_vision_tower / registry_e2e / audio_tower / paged_engine 1/0 each test_ltx2_text_encoder 17/3350 -> 23/3583 (+6 cases; +16 assertions with CHECKPOINT_ROOT, +6,289,222 with the e2e opt-in) FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…row/LTX25-L10-TEXT-TOWER
…e false Docs conflict was docs/FEATURES.md for the FOURTH time on the same keyed row (see #595). Took the campaign side wholesale: it is strictly newer and a superset, since L10's own text still said "STRUCTURAL e2e only, no render-quality claim", which stopped being true when L9C wired the connector. The substantive finding is in the merge, not the docs. ltx2_video.cpp:672 refuses a supplied encoder_path, and its reason is stated precisely: the Embeddings1DConnector's MATH is ported but its WEIGHTS are not loaded, they ship inside the DiT file, and Ltx2LoadDitFromSafetensors "still lists them among the modules it REFUSES". That was accurate when L10 was written against its base. It is NOT accurate on this merged tree. L9c landed exactly that loader: Ltx2LoadConnectorWeights is called at ltx2_video.cpp:792,794 and ltx2_loader.cpp:417 says in terms "NOT unported: phase L9c materializes them". So the two halves of prompt conditioning have met for the first time. L10 makes the tower RUN -- tokenizer token-exact on the shipped 262144-entry vocab, all 49 hidden states within the oracle's bf16 noise floor, both caption projections producing the 4096 and 2048 streams. L9c makes the connector those streams must pass through real, with the checkpoint's own weights. Neither branch alone could see that, which is exactly the class of thing a merge is supposed to surface and usually does not. Deliberately NOT lifting the refusal in this merge commit. Whether the last hop is complete is a question for a fresh implementer with a gate, not for the coordinating session to assert. The stale tensor-list comment at ltx2_loader.h:96-99 is owed the same correction. This is the third refusal in this campaign whose REASON went false while the refusal itself stayed correct. A refusal that names its cause is worth far more than one that does not -- but only if something re-reads the cause when the tree moves. Issue: #435 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…survive measurement (#435) Eight findings from the L10 fresh review. Six close as written; two close differently, because the thing they prescribed does not work and the oracle says so. Both are recorded here rather than quietly worked around, since an implementer who accepts a prescription that fails is the failure this protocol exists to catch. ──────────────────────────────────────────────────────────────────────────────── F3 — THE PRESCRIBED FIX IS REFUTED; THE ROPE TABLE IS THE INSTRUMENT The finding is real: `g.rope_partial_full = 1.0` (full rotary on the full- attention layers instead of the config's 0.25) left the suite green at 23 cases / 3583 assertions. Reproduced here before touching anything. The prescription was "a fixture whose global_head_dim/seq keep the rope contribution above the floor". MEASURED against the oracle, per state, as signal = max|bf16 @0.25 - bf16 @1.00| over floor = max|f32 - bf16|: committed head_dim 8/16 seq 8 worst signal/floor 1.095 wider full head head_dim 16/32 seq 8 0.258 longer seq head_dim 8/16 seq 24 0.764 wider + longer head_dim 16/32 seq 24 0.523 wider + longer head_dim 16/32 seq 32 0.650 Enlarging the fixture makes it WORSE. bf16 accumulation noise grows at least as fast as the rope contribution, so no reachable fixture size separates them: the hidden states are the wrong instrument, not a badly sized one. The right instrument is the table. `BuildProportionalRopeCache`'s host computation is split out and exported as `Gemma4ProportionalRopeCosSin`, the generator emits the oracle's own `Gemma4UnifiedTextRotaryEmbedding` cos|sin for the full-attention layer type (section 6), and the new case compares them in f32 with nothing accumulating. Measured agreement 1.267e-07 against a 1e-6 bound, plus an EXACT structural check that pairs at and beyond `int(partial*head_dim//2)` are cos=1, sin=0 and that the pairs below it really rotate. RED under the mutation: 202 assertions fail, worst 0.56533 vs 1e-6, exit 1 GREEN restored: 26 cases / 4115 assertions, exit 0 ──────────────────────────────────────────────────────────────────────────────── F2 — THE VALUE GATE AND THE LOADER, IN CI; AND M6 IS NOT WHAT IT LOOKED LIKE `Ltx2EncodePromptToConditioning`, `Ltx2LoadGemmaTowerFromSafetensors` and all four documented refusals were reachable from exactly ONE place — the opt-in 24 GB case — so none had CI coverage. They do now. The reduced tower fixture, which is already held to a running oracle, is written out as a real .safetensors under the CHECKPOINT's tensor names, loaded back, and compared to the fixture byte for byte; then a tokenizer whose added tokens spell the oracle's own token ids drives the whole prompt path and its conditioning is held to the committed LEFT-PADDED oracle run at a floor propagated through the identical projection. concat order q,k,v -> q,v,k RED 14 assertions, exit 1 PLE refusal disabled RED 1 assertion, exit 1 missing-v_proj refusal disabled RED 1 assertion, exit 1 module dtype F16 read as bf16 RED 1 assertion, exit 1 norm vector F16 read as bf16 RED 1 assertion, exit 1 conditioning at 0.565x / 0.688x of its propagated floor Now the part that did not survive. The review called `positions[i] = i` "the exact defect the surrounding comment warns about". It is not a defect at all. Rotary embedding is RELATIVE and the pads are masked out of attention, so shifting every position by the pad count cancels. MEASURED in the oracle, same 8 tokens told 12..19 and told 0..7: f32 max|diff| 5.11e-05 over max|value| 14.35 3.6e-06 relative bf16 max|diff| 1.09 0.65-1.70x the dtype floor f32 round-off, and at bf16 the rounding of different absolute angles. So the comment at ltx2_text_encoder.cpp — "a port that renumbers from zero rotates every query by the wrong angle" — was an overclaim, and so was its twin in the left-pad case. Both now carry the measurement. The numbering is still mirrored, because transformers derives positions from `cache_position` and fidelity is the reason; arithmetic is not. The new case does red under the renumbering, at 1.10x the audio floor, and says in its own comment that the narrow margin is a property of the defect rather than of the gate. ──────────────────────────────────────────────────────────────────────────────── F4 — THE GENERATOR'S LEGS WERE COUPLED, AND NOT WHERE IT LOOKED `run_tower` did `inner.to(dtype)`, which converts IN PLACE, so every leg after the bf16 one ran downstream of it. Each leg now runs on a `copy.deepcopy`. The mechanism is narrower than "bf16-rounded weights": MEASURED on this transformers build, the parameters round-trip unchanged, and it is the rotary embedding's non-persistent `inv_freq` BUFFERS that do not — 9.36e-05 on the full-attention table, 9.77e-05 on the sliding one. Regenerating moved 12 of the 13 padded f32 states (state 12 by 1.03e-02 on values of 2.705) and every entry of the pad-equivalence vector, while sections 2, 3 and 3b are byte-identical — which is exactly the partition the in-place conversion predicts. The header's claims about sections 4 and 5 are corrected to match. ──────────────────────────────────────────────────────────────────────────────── F5, F7, F8, F9 F5 337 -> 329, and pinned exactly rather than `> 300`. The count is arithmetic and now says so: 40*7 + 8*6 + 1. A lower bound is satisfied by a loader that quietly took a third of the layers from a bf16 fallback. F7 The rationale was inverted. Upstream calls `self.tokenizer(text, ...)` — `__call__` with its default `add_special_tokens=True` — so upstream DOES run the post-processor and we do not. Identical on this checkpoint because its `special_tokens` map is empty, measured. "THE TWO REFERENCES DISAGREE" also overstated it: both let the post-processor run, and only the explicit BOS prepend differs. Both corrected. F8 `kv.dtype` on the production prompt path was f32 where upstream resolves bf16 and where the attention itself runs bf16. Narrowed, not annotated. It was never wrong — it was WIDER, which is why nothing could see it: with the cache back at f32 the new conditioning gate reproduces 0.0617859 / 0.0394362 byte for byte. Half the cache bytes, and it stops taking `Gemma4AttnBlock`'s two-cast-buffers-plus-CastF32 arm on every layer. F9 The dropped `docs/FEATURES.md` edit is reapplied onto the campaign's rewritten row, truthfully: "Text tower RUNS" alongside the `encoder_path` refusal, which another change owns and this one does not touch. Row 533 chars, cells 60/28/217/215. `docs/USAGE.md` is owed too, and the debt turned out to be real rather than procedural: line 372 still said "the Gemma-4 12B text tower is not ported", which is false and which the paragraph 85 lines below it already contradicted. One document, two answers about one model. Corrected to state that the tower runs and to defer to that paragraph for what still blocks a prompt. The `encoder_path` passage itself is untouched — it belongs to the change that owns F1. ──────────────────────────────────────────────────────────────────────────────── NOT CLOSED HERE, AND OWED F1 and F6 are out of scope by instruction — another implementer owns whether the `encoder_path` refusal can be lifted now that L9c landed the connector weights. The spec is operator-owned, so this reports rather than edits it. Two things it needs. First, §4.2's open question stands and should not be narrowed by anything above: the shipped NVFP4 tower's output is still never compared to an independent oracle, so "all 49 hidden states within the oracle's bf16 noise floor" is proven for the SYNTHETIC reduced tower with bf16 weights and not for the quantized one that ships. PR #571 built the independent-oracle correlation technique that would close it. Second, the position-numbering claim appears in the spec in the same overclaiming form corrected here. ──────────────────────────────────────────────────────────────────────────────── GATE cmake -S . -B bld -G Ninja -DCMAKE_BUILD_TYPE=Release -DVLLM_CPP_BUILD_EXAMPLES=OFF cmake --build bld -j 12 BUILD_EXIT=0, no ENOSPC/BFD, df 88% ctest -N 409 tests ctest -j 8 --output-on-failure 409/409 passed, exit 0 ./bld/tests/test_ltx2_text_encoder 26 cases / 4115 assertions, exit 0 (baseline was 23 / 3583) `scripts/agent-preflight.sh` fails `doc-checkpoint range` on commits b0aa475, d67f812 and aa6aa0e. All three are ancestors of the branch head this work started from and none is touched here. Issue: #435 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…#435) The last round corrected the post_processor rationale in the HEADER and left the SOURCE saying the opposite thing, twelve lines above the `Encode` call it annotates. The two made contradictory predictions from the same premise: the header said a future post_processor makes us WRONG, the source said it keeps us RIGHT. F7's whole content was "the rationale was inverted", so half of it was still open. VERIFIED AGAINST UPSTREAM, not taken on trust. The reviewer could not reach the anchor; this was read at /home/mudler/_git/LTX-2 pin fd4ded7f: packages/ltx-core/src/ltx_core/text_encoders/gemma/tokenizer.py:37-43 encoded = self.tokenizer( text, padding=False, truncation=True, max_length=self.max_length, return_tensors="pt", ) That is `__call__` with its DEFAULT `add_special_tokens=True`, so upstream DOES run the post_processor. The HEADER had the direction right; the SOURCE was the wrong one, and it is the source that is corrected here. A THIRD claim in the same comment was also false. "Doing both would double the BOS" is not true: upstream's own guard at :45 (`not input_ids or input_ids[0] != bos_id`), mirrored at ltx2_text_encoder.cpp:712, absorbs a leading BOS from either call. The real exposure is a post_processor that adds anything ELSE, and the comment now says that instead. SIBLING SWEEP, built from the SUBJECT (post_processor, BOS, add_special_tokens, Encode) rather than from the sentences being fixed, since a grep built from the new text only confirms itself. Two more hits, both corrected: * gen-ltx2-prompt-tokens-goldens.py:86 annotated its `add_special_tokens=False` as transcribing ":38-43" -- the same conflation, in the generator that produces the committed goldens. Anchor corrected to :37-43 and the deliberate flag flip is now stated. * ltx2_text_encoder.h:421 called the BOS prepend "unconditional", which contradicts test_ltx2_text_encoder.cpp:2398 ("i.e. conditional") and :45 itself. It is conditional, and the guard is why. ──────────────────────────────────────────────────────────────────────────────── TWO COMMENT OVERCLAIMS, PROVED BY THE REVIEWER'S MUTATION Replacing `Ltx2ComputeRightPadOrder`'s stable partition with the identity permutation reds two OTHER cases (22 assertions) and leaves the new conditioning case entirely green. `out.conditioning` and `want_f32` both reach `sort_index` and `additive_mask` through the SAME function over the SAME `out.mask`, so a defect there cancels and those ~40 assertions cannot fire. "Held EXACTLY" was an assertion true by construction wearing a gate's label. This is NOT a coverage hole and no test changes. Both are genuinely gated by "ltx2 text: additive mask, right-pad ordering and the binary mask" and "ltx2 text: the encoder -> conditioning hand-off", which compare against committed upstream goldens rather than against a second call of the code under test. The comment now says what its assertions do establish (the mask is threaded through unaltered, and the mask itself is held to the golden) and names the two cases that actually gate the ordering. ──────────────────────────────────────────────────────────────────────────────── TWO COVERAGE LIMITS RECORDED, NOT FIXED * The loader case's "the q|k|v concat ORDER" claim holds for the LTX path ONLY. `Ltx2LoadGemmaTowerFromSafetensors` uses its own `TowerConcat` (ltx2_text_encoder.cpp:943-945); `gemma4_weights.cpp:281-295` is a SEPARATE implementation this suite never loads, and mutating it leaves every case here green. That path owes its own gate. * The rope-table instrument covers the FULL-ATTENTION arm only. The sliding layers' `default` rope at theta 1e4 has no equivalent f32 instrument and is reached only through the hidden states, where the same bf16 noise-floor argument works against resolving a config-carried angle defect. ──────────────────────────────────────────────────────────────────────────────── DOCS Comment-only in src/include/tests, so AGENTS.md owes nothing here -- but check-doc-checkpoint is path-derived and demands FEATURES.md (from src/vllm/model_executor/models/) and USAGE.md (from include/vllm/). The prior repair at 9fbc682 paid it the same way. Both edits are GENUINE rather than gate-appeasement: they record the tokenization divergence this commit just verified, which is a real user-facing caveat -- our prompt tokenization mirrors upstream only while the checkpoint's post_processor stays empty. USAGE.md:454 and :473 are left alone; they are inherited from the base branch and belong to PR #600's lane. The FEATURES.md note is folded into an existing paragraph rather than added as a new one, because check-public-doc-tables budgets prose paragraphs (21) as well as table cells (220 chars) and rows (600), and the LTX row's cell is already at 217. CODE IS BYTE-IDENTICAL. The only non-comment line in the diff is a trailing comment on an unchanged statement. GATE, CPU, Release, VLLM_CPP_CUDA=OFF: BUILD_EXIT=0, no "No space left" / "BFD assertion", df 86% used test_ltx2_text_encoder: 26 cases / 4115 assertions, Status SUCCESS, exit 0 (matches the declared baseline exactly) ctest -N: 414 (the brief said 409; this configure enumerates 414 at the unmodified head, and a comment-only diff cannot register a test) ctest: 414/414 passed, 0 failed, exit 0, first pass under -j 8, no flaky re-runs needed binary md5 unchanged across the post-docs rebuild (8b48f39b...) check-doc-checkpoint "range" still reports b0aa475, d67f812 and aa6aa0e. All three are ancestors of 9fbc682, inherited, and not repairable from here without editing #600's lane. Issue: #435, #604 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
, #604) The re-review found `test_ltx2_text_encoder.cpp:635` citing `ltx2_text_encoder.cpp:943-945` for "the LTX tower loader's own `TowerConcat`". Lines 943-945 are the `q_proj` `TowerModule` load. The concat calls are :951 and :953; the helper is defined at :835. The substantive claim was measured and is true, and only the number was wrong -- which is the worst shape for this defect, because a reader who follows the anchor concludes a TRUE claim is unsupported. Fixing one number and stopping is what let the class survive three review rounds, so every `file:line` this branch ADDS over `row/MODEL-DIFFUSION-LTX25` was resolved against the tree it points into. Extracted mechanically by SHAPE off the added lines of `git diff origin/row/MODEL-DIFFUSION-LTX25...HEAD` (`name.ext:NN`, `name.ext:NN-MM`, bare `:NN` / `:NN-MM`, "line(s) NN", "SS N"), not by grepping the sentences already known to be wrong -- a grep built from text you just fixed only confirms itself. 19 citations added, in 6 files. 19 resolved. 8 wrong, at 6 comment sites. site cited is ------------------------------------------------------------------------ test_ltx2_text_encoder.cpp:635 ltx2_te.cpp:943-945 q_proj load test_ltx2_text_encoder.cpp:1959 rope_utils.py:187-245 ends at 254 test_ltx2_text_encoder.cpp:1992 rope_utils.py:233 blank line gemma4.h:260 gemma4_unified.py:257-274 prev fn tail gemma4.h:260 rope_utils.py:187-245 ends at 254 gen-ltx2-gemma-tower-goldens.py:378 gemma4_unified.py:214-218 call, no lookup gen-ltx2-gemma-tower-goldens.py:378 rope_utils.py:187-245 ends at 254 ltx2_text_encoder.cpp:1055 gemma4.cpp:300-312 cuts the cast The three `modeling_rope_utils.py:187-245` sites are one anchor repeated. The function is `_compute_proportional_rope_parameters`, :187-254, and all three sentences name its ZERO PADDING -- which is `torch.zeros(nope_angles, ...)` at :246, ONE LINE past the cited end. Following the anchor is following it to the `torch.cat(` that opens two lines earlier and stopping before the argument that matters. Corrected to :187-254, with :246 named where the padding is the claim. `modeling_rope_utils.py:233` is a blank line. `rope_angles = int(rope_proportion * head_dim // 2)`, which the comment quotes, is :234. `modeling_gemma4_unified.py:257-274` opens on the PREVIOUS function's `return` (:257) and stops before `forward`'s own (:275). The `emb = torch.cat((freqs, freqs), dim=-1)` the sentence mirrors is :271, so the anchor does contain it, but both ends land in the wrong construct. Corrected to the whole method, :259-275, with :271 named. `modeling_gemma4_unified.py:214-218` is the CALL, `rope_init_fn(self.config, **rope_init_fn_kwargs)`. The sentence claims the class "routes `rope_type: proportional` to `_compute_proportional_rope_parameters`", and the routing is `ROPE_INIT_FUNCTIONS[rope_type]` at :207, outside the range -- a reader at :214-218 cannot see what `rope_init_fn` resolves to, so the claim reads unsupported. Widened to :206-218. `gemma4.cpp:300-312` opens on the weight-less V `RmsNorm` at :300 and closes on the `} else {` at :312, which EXCLUDES the `vt::CastF32` pair at :313-314 that the sentence is about ("run a CastF32 on every K and V it wrote"). The `kv.dtype != adt` arm is :306-315. Corrected, with the two `DBuf`s and the casts named separately, because those are the two costs the sentence charges. The eleven that resolve as written, checked one by one, not assumed: LTX-2 `tokenizer.py:37-43` (three sites, the `self.tokenizer(...)` call), `:44-46`, `:45`, `:12-15`, `gemma_assets.py:335-386` (`build_text_encoder_tensors_from_gemma_root`, :335 to its `return` at :386), `base_encoder.py:41`; diffusers `pipeline_ltx2.py:339` (literally `add_special_tokens=True`); ours `gemma4_weights.cpp:281-295`, which does span all three qkv arms as the previous round reported. Two non-line anchors added by the F7 commit -- the two test-case NAMES it cites as the ones that gate the ordering -- exist verbatim at :1115 and :1166. PINS. LTX-2 at `fd4ded7f`, tree clean. diffusers at `3a2f35d4`. transformers 5.12.1 at `/home/mudler/recon-cpu/venv`, the interpreter the tower generator actually ran under; both files are md5-IDENTICAL to `/home/mudler/_git/ transformers` @ `7d06b1a5`, so the numbering above is not an artifact of which install was read. F10's anchor. `:951-953` and not `:835`, because the sentence is the LTX half of a contrast whose other half is `gemma4_weights.cpp:281-295` -- assembly SITES, not a helper definition -- and because the qkv concat ORDER is what the case gates and what the previous round's mutation (`q,k,v` -> `q,v,k`, 14 assertions RED) perturbed at :951. `:835` is named alongside it so the helper is still one hop away. DOCS. The code is byte-identical; this is comments and two documents. `check-doc-checkpoint` is path-derived and demands `FEATURES.md` (from `src/vllm/model_executor/models/`) and `USAGE.md` (from `include/vllm/`), which is the same bill `9fbc68256` and `ddfcb516a` paid. Both edits are genuine. `USAGE.md` still opened its LTX-2.5 gate section with "**There is no LTX-2.5 render path yet** ... no text encoder, no VAE, no pipeline and no `/v1/videos` route for it -- asking the video engine for LTX-2.5 will not work." Every clause is false, and the SAME DOCUMENT contradicts all of them: `ltx2-gen` renders end to end at :412, `--video-family ltx-2.5` pins the family on the server at :435, and the text tower is this row. Rewritten to say what the section is for. The refusal list that follows is left byte-for-byte alone: it is inherited text, it is not part of this finding, and `ltx2.h:38` and `:47-49` do support the two clauses of it I checked. `FEATURES.md` pointed at "`ltx2_text_encoder.cpp` names the line that would have to change" -- an unresolvable pointer, in a commit about unresolvable pointers. It now names `Ltx2TokenizeGemmaPrompt`. A symbol, deliberately, because a line number in a public document has nothing that can keep it true. Folded into the existing paragraph: the 700-char prose cap and the 220-char cell cap make an addition an EVICTION of someone else's content, and the LTX row's two cells are already at 217 and 215. `docs/USAGE.md:454` and `:473` untouched -- #600's lane. GATE -- CPU, Release, `VLLM_CPP_CUDA=OFF`, `build-f10`, deleted after. cmake --build build-f10 -j 16 BUILD_EXIT=0; 0 hits for 'No space left|BFD assertion'; df 85% ctest -N 414 ctest -j 8 --output-on-failure 414/414 passed, 0 failed, exit 0 ./build-f10/tests/test_ltx2_text_encoder 26 cases | 4115 assertions | 0 failed Status: SUCCESS, exit 0 (baseline 26 / 4115 matched exactly) First pass under `-j 8`; no suite needed a serial re-run. `doc-checkpoint --staged` and `now-current --staged` both ok. `doc-checkpoint range` still reports `b0aa475a3`, `d67f8125e` and `aa6aa0ecd` -- the identical three failures this worktree recorded BEFORE any edit, all ancestors of `9fbc68256`, inherited and not repairable from here. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
Collaborator
Author
|
Landed on Reviewed PASS at The last round is the one worth reading. Told to fix ONE wrong Three of the eight are the same anchor repeated: The reviewer then re-extracted every citation independently, confirmed the sweep missed none, resolved all eight corrections itself at the pins, verified the two Two refutations from earlier rounds stand, both measured, both against instructions I gave:
Also closed by narrowing rather than widening: the f32 KV cache on the prompt path became bf16 and the gate reproduced byte for byte — never wrong, just wider, which is exactly why nothing could see it, and precisely the too-WIDE dtype trap a token gate structurally cannot catch. Closing; it reaches |
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
2 participants
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.
Phase L10 of the LTX-2.5 campaign (#435): make a text prompt actually work.
Branched from
row/MODEL-DIFFUSION-LTX25, merged back up to3f5ae3f45.What changed
LTX-2.5 advertises text-to-video and could not be typed at. This makes the prompt path run end to end: a real string tokenizes with the tokenizer the checkpoint ships as a tensor, the torchao-NVFP4 Gemma-4 tower is materialized onto
Gemma4Weightsand forwarded, all 49 hidden states feed the existing aggregation, and both caption projections emit the 4096-wide video and 2048-wide audio conditioning.The headline finding
L3's "the tower cannot be gated" was a
transformersversion, not a property of the tower. Measured both ways on this box: 5.3.0 raisesKeyError 'gemma4_unified'; 5.12.1 builds and runs it.Gemma4Model::ForwardHiddenStatesis now held to a running upstream for the first time —gemma4.hhas said "grounded + compiles" since it landed.The tolerance is a measurement, not a constant: the generator measures how far upstream's own answer moves between f32 and bf16, per state, and that is the bound. Worst state reaches 0.71x of its own floor; state 0 is bit-identical. We are closer to upstream-in-bf16 than upstream-in-bf16 is to upstream-in-f32, at every layer.
Mutation-proven: dropping
layer_scalar→ 34 assertions RED; capturing the raw last layer instead of the final-normed one → 3 RED. Tree restored byte-for-byte after each.Measured end to end on the real 12B checkpoint
Tokenization is token-exact against HuggingFace on the shipped 262144-entry vocab: four prompts x 1024 positions, first mismatching index
-1on all four.What is deliberately NOT done
has_encoder()is still false, and that is the honest answer rather than an omission. Upstream routes each stream through anEmbeddings1DConnectorbefore cross-attention. Its math is ported (Ltx2ConnectorForward); its weights are not loaded — they ship inside the DiT file asvideo_embeddings_connector.*/audio_embeddings_connector.*and remain among the modulesLtx2LoadDitFromSafetensorsrefuses. So the conditioning has nowhere to go, andtruewould promise a render that cannot complete.The refusal therefore moves rather than lifts:
encoder_pathis still refused, with a message that now names the connector weights instead of the tower, because the old message is no longer true. Flipping the flag is one change away, inltx2_loader.*/ltx2_video.cpp, which this row does not own.Baselines — all matching, none moved
test_ltx229/1615 ·test_ltx2_vae16/1816 ·test_ltx2_pipeline35/2358 ·test_ltx2_loader20/2363 ·test_ltx2_video17/170 ·test_ops_attention_cross9/32 ·test_minimax_h379/57395 ·test_minimax_h3_video_fold6/137 ·test_video_engine11/254 ·test_capi55/505Gemma-4, behaviour-unchanged:
test_gemma4_honesty2/6 ·test_gemma4_rocm_fp8_seams2/10 ·test_gemma3_forward3/503 ·test_tool_parser_gemma472/161 ·test_gemma4_{vision_tower,registry_e2e,audio_tower,paged_engine}1 case / 0 assertions eachtest_ltx2_text_encoder17/3350 → 23/3583 → 26/4115 (review round 1 added the rope table, the CI-reachable loader with six refusal subcases, and the prompt→conditioning value gate); 4131 withCHECKPOINT_ROOT, and the e2e opt-in on top.Review round 1 — eight findings, at
9fbc68256Six close as written. Two do not, and the oracle is why; both are argued in the commit message rather than worked around.
partial_rotary_factorunreachabledocs/FEATURES.mdeditdocs/USAGE.mdhad a real contradiction tooF3's prescription does not work. Measured against the oracle,
signal = max|bf16 @0.25 − bf16 @1.00|overfloor = max|f32 − bf16|, worst over 13 states:Enlarging the fixture makes it worse — bf16 accumulation noise grows at least as fast as the rope contribution. The hidden states are the wrong instrument, not a badly sized one. So
BuildProportionalRopeCache's host computation is exported asGemma4ProportionalRopeCosSinand compared in f32 to the oracle's ownGemma4UnifiedTextRotaryEmbedding(new golden section 6): agreement 1.267e-07 against a 1e-6 bound, plus an exact structural check on the zero-padded pairs.M6 is not a defect. RoPE is relative and the pads are masked, so a uniform position shift cancels. In the oracle, the same 8 tokens told
12..19and told0..7:The comments claiming renumbering "rotates every query by the wrong angle" — in
ltx2_text_encoder.cppand in the left-pad test — were overclaims and now carry that measurement. The numbering stays mirrored for fidelity to transformers, not for arithmetic.Mutations, all reproduced
rope_partial_full = 1.0(M3)positions[i] = i(M6)q,k,v→q,v,kv_projrefusal disabledGate
test_cpu_x86_llamacpp_floorfailed once under load 46 withwaiting for quiet: busy=107%, then passed 10/10 serially at load 29. Contention, not this change.Still owed, for the operator
.agents/specs/ltx-2-5.mdis operator-owned and was not edited. It needs two things. §4.2's open question stands and nothing here narrows it: the shipped NVFP4 tower's output is still never compared to an independent oracle, so "all 49 hidden states within the oracle's bf16 noise floor" is proven for the synthetic reduced tower with bf16 weights, not for the quantized one that ships. PR #571 built the independent-oracle correlation technique that would close it. Separately, the position-numbering claim appears in the spec in the same overclaiming form corrected here.Review notes
.agents/specs/ltx-2-5.mdhas no L10 row — the spec is the operator's and was not edited. The phase brief stood in for it, which is a departure from "spec before code" that the operator should reconcile.doc-checkpoint rangestill reportsb0aa475a3andd67f8125e— both pre-existing on the base branch, neither from this row.Re-review repair — head
ddfcb516a(#435, #604)The re-review returned FAIL on one finding (F7), closed everything else, and confirmed both of the previous round's refutations. Comment accuracy only: the code is byte-identical, the only non-comment line in the diff being a trailing comment on an unchanged statement.
F7: CLOSED — and the previous round had the direction RIGHT
The header correction landed at
ltx2_text_encoder.h:427and left the source atltx2_text_encoder.cpp:691saying the opposite thing, twelve lines above theEncodecall it annotates. Same premise, contradictory predictions.The reviewer could not reach the upstream anchor, so which one was correct was taken on trust. Verified here at
/home/mudler/_git/LTX-2, pinfd4ded7f:packages/ltx-core/src/ltx_core/text_encoders/gemma/tokenizer.py:37-43That is
__call__with its defaultadd_special_tokens=True, so upstream does run the post_processor. The header was right; the source was the inverted one, and the source is what changed.A third claim in that same comment was also false: "Doing both would double the BOS". Upstream's own guard at
:45(not input_ids or input_ids[0] != bos_id), mirrored atltx2_text_encoder.cpp:712, absorbs a leading BOS from either call. The real exposure is a post_processor that adds anything else.Sibling sweep
Built from the subject (
post_processor, BOS,add_special_tokens,Encode), not from the sentences being fixed. Two further hits, both corrected:scripts/gen-ltx2-prompt-tokens-goldens.py:86add_special_tokens=Falseas transcribing:38-43— the same conflation, in the generator that produces the committed goldens. Anchor corrected to:37-43; the deliberate flag flip is now statedltx2_text_encoder.h:421test_ltx2_text_encoder.cpp:2398("i.e. conditional") and:45itselfThe two overclaims — wording fixed, tests untouched
Replacing
Ltx2ComputeRightPadOrder's stable partition with the identity permutation reds two other cases (22 assertions) and leaves the new conditioning case entirely green.out.conditioningandwant_f32both reachsort_indexandadditive_maskthrough the same function over the sameout.mask, so a defect there cancels and those ~40 assertions cannot fire. "Held EXACTLY" was an assertion true by construction wearing a gate's label.Not a coverage hole, and no test changed. The comment now states what the assertions do establish — the mask is threaded through unaltered, and the mask itself is held to the golden above — and names the two cases that actually gate the ordering: "ltx2 text: additive mask, right-pad ordering and the binary mask" and "ltx2 text: the encoder -> conditioning hand-off", both against committed upstream goldens rather than a second call of the code under test.
Coverage limits recorded (not fixed)
Ltx2LoadGemmaTowerFromSafetensorsuses its ownTowerConcat(ltx2_text_encoder.cpp:943-945);gemma4_weights.cpp:281-295is a separate implementation this suite never loads, and mutating it leaves every case here green. That path owes its own gate.defaultrope at theta 1e4 has no equivalent f32 instrument and is reached only through the hidden states, where the same bf16 noise-floor argument works against resolving a config-carried angle defect.Docs
Comment-only in
src//include//tests/owes nothing under AGENTS.md, butcheck-doc-checkpointis path-derived and demandsFEATURES.md(fromsrc/vllm/model_executor/models/) andUSAGE.md(frominclude/vllm/);9fbc68256paid it the same way. Both edits are genuine rather than gate-appeasement — they record the tokenization divergence this commit just verified, which is a real user-facing caveat.docs/USAGE.md:454and:473were left alone: inherited from the base branch, PR #600's lane.The
FEATURES.mdnote is folded into an existing paragraph becausecheck-public-doc-tablesbudgets prose paragraphs (21) as well as cells (220 chars) and rows (600) — the LTX row's cell is already at 217.Gate — CPU, Release,
VLLM_CPP_CUDA=OFFFirst pass under
-j 8; no flaky suite needed a serial re-run.ctest -Nenumerates 414, not the 409 in the brief — that count comes from this configure at the unmodified head, and a comment-only diff cannot register a test.doc-checkpoint rangestill reportsb0aa475a3,d67f8125eandaa6aa0ecd. All three are ancestors of9fbc68256, inherited, and not repairable from here without editing #600's lane.🤖 Generated with Claude Code
F10 and the anchor sweep — head
131328e86(#435, #604)The third review returned one finding, F10:
test_ltx2_text_encoder.cpp:635citedltx2_text_encoder.cpp:943-945for "the LTX tower loader's ownTowerConcat". Lines 943-945 are theq_projTowerModuleload. F10 is CLOSED, and it was not alone.Fixing one number and stopping is what let this class survive three rounds, so every
file:linethis branch adds overrow/MODEL-DIFFUSION-LTX25was resolved against the tree it points into — extracted mechanically by SHAPE off the added lines of the branch diff (name.ext:NN,name.ext:NN-MM, bare:NN/:NN-MM, "line(s) NN"), never by grepping sentences already known to be wrong.19 citations added, in 6 files. 19 resolved. 8 wrong, at 6 comment sites.
test_ltx2_text_encoder.cpp:635ltx2_text_encoder.cpp:943-945q_projTowerModuleload:951-953(+ helper at:835)test_ltx2_text_encoder.cpp:1959modeling_rope_utils.py:187-245:187-254; the pad is:246:187-254test_ltx2_text_encoder.cpp:1992modeling_rope_utils.py:233:234:234gemma4.h:260modeling_gemma4_unified.py:257-274return:259-275(catat:271)gemma4.h:260modeling_rope_utils.py:187-245:187-254gen-ltx2-gemma-tower-goldens.py:378modeling_gemma4_unified.py:214-218:206-218(lookup at:207)gen-ltx2-gemma-tower-goldens.py:378modeling_rope_utils.py:187-245:187-254ltx2_text_encoder.cpp:1055gemma4.cpp:300-312CastF32pair it names:306-315Three of the eight are one anchor repeated.
_compute_proportional_rope_parametersis:187-254, and all three sentences name its zero padding —torch.zeros(nope_angles, ...)at:246, one line past the cited end. The anchor stops inside thetorch.cat(argument list, before the argument that is the whole claim.gemma4.cpp:300-312is the same shape in our own tree: it opens on the weight-less VRmsNormand closes on the} else {, so thevt::CastF32pair at:313-314that the sentence charges is outside it. Thekv.dtype != adtarm is:306-315.modeling_gemma4_unified.py:214-218showsrope_init_fn(...)but notROPE_INIT_FUNCTIONS[rope_type]at:207, so a reader cannot confirm the word "routes".F10's anchor is
:951-953, not:835. The sentence is the LTX half of a contrast whose other half isgemma4_weights.cpp:281-295— assembly sites, not a helper definition — and the qkv concat ORDER is what the case gates and what round 1'sq,k,v→q,v,kmutation (14 assertions RED) perturbed, at:951.:835is named alongside so the helper stays one hop away.The eleven that resolve as written, checked individually:
tokenizer.py:37-43(3 sites),:44-46,:45,:12-15,gemma_assets.py:335-386,base_encoder.py:41,pipeline_ltx2.py:339,gemma4_weights.cpp:281-295. Plus two non-line anchors from the F7 commit — the two test-case NAMES it cites — present verbatim at:1115and:1166.Pins. LTX-2
fd4ded7f(clean), diffusers3a2f35d4, transformers 5.12.1 at/home/mudler/recon-cpu/venv— both files md5-identical to/home/mudler/_git/transformers@7d06b1a5, so the numbering is not an artifact of which install was read.Docs
Code is byte-identical; this is comments plus two documents.
check-doc-checkpointis path-derived and billsFEATURES.md+USAGE.md, the same bill9fbc68256andddfcb516apaid. Both edits are genuine, and one is a real defect:USAGE.mdstill opened its LTX-2.5 section with "There is no LTX-2.5 render path yet … no text encoder, no VAE, no pipeline and no/v1/videosroute for it — asking the video engine for LTX-2.5 will not work." Every clause is false and the same document contradicts all of them (ltx2-genrenders e2e at:412,--video-family ltx-2.5at:435, and the text tower is this row). Rewritten to state what the section is for. The inherited refusal list that follows is left byte-for-byte alone.FEATURES.mdpointed at "ltx2_text_encoder.cppnames the line that would have to change" — an unresolvable pointer, in a commit about unresolvable pointers. It now namesLtx2TokenizeGemmaPrompt, by symbol, because a line number in a public doc has nothing to keep it true. Folded into the existing paragraph: the 700-char prose cap and 220-char cell cap make an addition an eviction, and the LTX row's cells are at 217 and 215.docs/USAGE.md:454/:473untouched — #600's lane.Gate — CPU, Release,
VLLM_CPP_CUDA=OFFFirst pass under
-j 8; no suite needed a serial re-run. Build dir deleted.doc-checkpoint --stagedandnow-current --stagedboth ok;doc-checkpoint rangestill reportsb0aa475a3,d67f8125e,aa6aa0ecd— the identical three this worktree recorded before any edit, inherited, not repairable from here.