Repository navigation
Inkling attn backend cleanup fake registry - #17860
Draft
KleinBlueC wants to merge 94 commits into
Draft
KleinBlueC wants to merge 94 commits into
KleinBlueC wants to merge 94 commits into
Conversation
Add the Inkling NVFP4 model to the _torch stack: modeling_inkling.py, a Triton score_mod attention backend (SWA + global + relative-position bias) over KVCacheManagerV2, HF NVFP4 weight mapper/configs, trtllm-gen blockScaleMoe runner with sink-renorm routing, reasoning-parser/effort rendering, lm_eval post-processing, and the inkling_* unittest suite. Progress (working snapshot): - Component validation passes in isolation on the TP=4 NVFP4 / trtllm-gen MoE stack: weight load & accounting, attention source-activation replay, MoE replay, and full-model source-logit replay. - Baseline (cuda_graph=off, overlap=off) accuracy vs the SGLang reference: GSM8K 0.916 vs 0.972 (-5.6pt), full MMLU 82.22 vs 85.66 (-3.44pt). The gap is dominated by runaway / non-terminating generation on hard prompts: 76% of GSM8K errors are 7k-8.6k-token spirals where the model reaches the answer but never emits EOS, while SGLang commits. Per-layer localization is exhausted; the residual is a diffuse fp4 / kernel-family divergence (bf16 Triton attention + trtllm-gen fp4 MoE vs SGLang flashinfer), not a single fixable layer bug. - Enabled (cuda_graph=on) is blocked by a decode collapse (B2) localized to the global-attention block under CUDA-graph capture/replay at TP=4: h_attn goes non-finite at the first global-attention layer once decode crosses a KV-page boundary; a reduced-model TP=2 harness reproduces it. Next: decode-side termination fix for the baseline runaway (finish_reason-based detection + EOS/stop handling), and resolve B2 in the global-attention-under- graph path. Excludes build artifacts (libtensorrt_llm.so), locks, and the ext/ submodule. Signed-off-by: kleinc <kleinc@nvidia.com>
…(6*448)) Root cause of the baseline NVFP4 accuracy gap vs SGLang. The Inkling checkpoint ships routed-expert activation calibration as a RAW `.input_amax`, but the fused-MoE loader / trtllm `fp4_quantize` expect the ModelOpt per-tensor `input_scale = amax / (E2M1_MAX * E4M3_MAX) = amax / (6*448)`. The mapper renamed `.input_amax` -> `input_scale` WITHOUT the conversion, making the activation global scale 2688x too small: every routed expert's fp4 output was ~0.62 rel_rms from the bf16 ground truth (7.6x SGLang's 0.082). Mirrors sglang inkling.py:1222 / inkling_common/dense_mlp.py:497. Positional bisection confirmed the weight/block-scale/gate-up-interleave layout was already element-wise correct (24/24 L3 experts, 18.8M elems, 0 diverged) -- the defect was ONLY the activation input scale, shared by both TRT MoE backends. Fix: `inkling_weight_mapper._map_expert` divides `.input_amax` by (6*448). Result (baseline: cuda_graph=off, overlap=off, TP=4, CUTLASS MoE, fp4 fix active): - L3 routed-expert vs bf16 truth: rel_rms 0.624 -> 0.081 (== SGLang 0.082) - GSM8K paired 5x100: TRT 0.916 -> 0.968 vs SGLang 0.974; mean_delta -0.056 -> -0.006 (Gate 2 PASS, within +/-0.02); runaway canary passes; collapse 0 - MMLU paired 6x100: TRT ~0.822 -> 0.862 vs SGLang 0.872; gap -3.44pt -> -1.0pt (Gate 3 accuracy PASS); B-bias error-is-B 65.9% -> 40%, TRT-B 33.2% -> 29.8% Residual (not this fix): TRT (CUTLASS) vs SGLang (flashinfer) fp4 kernel-family near-tie noise -- accuracy-neutral, not bit-reproducible at batch>1. The strict within-2pp-of-gold MMLU B-bias criterion is model-inherent (SGLang itself over-picks B) and is left to human adjudication, not a TRT defect. Also adds env-gated per-layer/per-module dump instrumentation (modeling_inkling.py dump_sink; inkling_perlayer_localize_test.py) and the trtllm-gen MoE backend path used to localize the bug. Signed-off-by: kleinc <kleinc@nvidia.com>
…ring graph capture
Root cause of the Inkling TP=4 enabled-runtime (cuda_graph=on + overlap) decode collapse
("B2"): the AutoTuner-selected all-reduce (`tunable_allreduce`, AUTO strategy) is not
CUDA-graph-capture-safe. Frozen into a decode graph, the tuned tactic produces a non-finite
result on replay, so decode goes NaN from the first global-attention layer and collapses to
a token-0 repeat ("Paris!!!!"). Eager is finite at identical metadata; the fault is baked
into the captured graph.
Localized by single-variable determinism isolation (autotuner ON -> collapse, autotuner OFF
-> clean), NOT by op-fingerprints -- any in-graph probe shifts the graph memory pool / tactic
selection and suppresses the bug (a Heisenbug).
Fix (`tensorrt_llm/_torch/distributed/ops.py`, AllReduce.forward): during CUDA-graph capture
(`torch.cuda.is_current_stream_capturing()`), skip `tunable_allreduce` and fall back to the
static, graph-safe `all_reduce_op(AUTO)`. Warm-up / eager (not capturing) still autotune, so
steady-state performance is unchanged. This is a mainline TRT-LLM robustness fix (any TP
model that captures an AUTO all-reduce benefits), not Inkling-specific.
Confirmed: enabled generation_parity is BIT-IDENTICAL to the baseline (cuda_graph=off) --
tf_mismatch/neartie/confident=16/11/5, freerun_collapse=0, identical logit_checksum; enabled
5x100 GSM8K (0.966 vs SGLang 0.974) and MMLU (0.875 vs 0.872) within +/-0.02, zero errors,
no regression vs the accepted CUTLASS baseline.
Also includes env-gated per-layer/per-op B2 localization instrumentation
(`modeling_inkling.py` dump_sink / INKLING_FP*; `inkling_fp_localize_test.py`), zero-cost
when the INKLING_FP* env is unset.
Signed-off-by: kleinc <kleinc@nvidia.com>
…MLP tower, image fusion (WIP) WIP -- Stage-1 progress snapshot on the Inkling NVFP4 multimodal tower, on top of the accepted text tower. Not finished: the vision path is verified clean but the MMMU Accounting gap is still open (decode-side), and audio / MTP are still deferred. Committed to record current progress, not as a complete feature. Text decode/attention/MoE paths are untouched. Model / config: - configs/inkling.py: add image_token_id (200054, the in-vocab chat-template <|unused_200054|>) and audio_token_id. The SGLang-internal -101 sentinel is rejected by TensorRT-LLM's executor token-id validation; the two ids are interchangeable for parity since both are overwritten by vision embeddings. - modeling_inkling_vision.py (new): hMLP vision tower InklingVisionModel and InklingInputProcessor, which expands the <image> placeholder to one token per vision patch and attaches vision_patches_bthwc features. - modeling_inkling.py: InklingForConditionalGeneration registers the input processor, builds the vision tower as a replicated bf16 submodule, and fuses per-patch embeddings into the text stream via fuse_input_embeds with explicit text/mm indices (OOV-safe). Fixes the "image not visible" hallucination: the fused stream must NOT be re-normed, since SGLang scatters raw vision rows in after embed_norm; the extra RMSNorm corrupted the image rows. Tests / diagnostics (tests/unittest/_torch/modeling/, 25 new files): MMMU harness alignment against the SGLang scorer, input-processor and vision-tower unit checks, image e2e / fusion / logit-replay / generation-parity drivers, and the localization probes used to isolate the vision-vs-decode split (vision verified bitwise-clean; the residual Accounting gap is decode-side). These require GPU + the NVFP4 checkpoint (TP=4) and were not run for this commit. Signed-off-by: kleinc <kleinc@nvidia.com>
…s_token SamplingParams._setup() looked for the end-of-generation token in only two places: tokenizer.eos_token_id, then generation_config.eos_token_id. When a checkpoint provides neither, end_id stayed None, nothing could terminate a request, and every generation silently ran to max_tokens. That combination is reachable. Multi-part chat formats have no single terminator -- a message end, an end-of-sampling marker and a document separator are distinct tokens -- so such checkpoints register their control tokens under extra_special_tokens / additional_special_tokens, neither of which populates tokenizer.eos_token_id, and declare the real stop token as eos_token_id in config.json. Those checkpoints also tend to ship no generation_config.json. _setup() already received hf_model_config but never consulted it. Observed on a checkpoint of that shape: responses ran to the token limit while emitting the configured eos_token_id up to 441 times in a single response. Fall back to hf_model_config.eos_token_id when the first two sources yield nothing. A list value sets end_id from its first entry and appends the rest to stop_token_ids, mirroring the existing generation_config path. Priority is otherwise unchanged: an explicit SamplingParams(end_id=...) still wins and tokenizer.eos_token_id still takes precedence over config.json, so models that already resolved an end_id see no behavioural change. Warn when all three sources come up empty. Generating to the cap on every request with no diagnostic is the part that makes this expensive to find. Add unit tests for tokenizer priority, the config fallback, the list form, an explicit end_id not being overridden, the all-empty case, and a missing hf_model_config. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Adds the audio and video modalities alongside the existing vision path.
Audio:
* InklingAudioPreprocessor -- dMel feature extraction (mel basis, hz<->mel,
per-frame bin quantization) producing int32 [N, 80] bins, one audio token
per frame.
* InklingAudioModel -- codebook encoder (bin m occupies codebook rows
[m*V, (m+1)*V), summed over bins) plus optional final norm, bf16, loaded
strictly from the real checkpoint's audio tensors.
* Placeholder expansion and fail-loud count checks in the input processor.
Video:
* sample_video_frames / sample_video_as_images / DecodedVideo -- frame
sampling ported to match SGLang's sample_video_frames semantics.
* The <image>-per-frame path: every sampled frame becomes its own image
span through the existing vision tower.
Audio and video land together because both wire into the same
InklingInputProcessor.assemble dispatch; splitting them would leave an
intermediate commit whose processor references helpers that do not exist yet.
Tests (all GPU-verified on TP=4):
* inkling_audio_tower_test.py -- 10 passed, incl. real-weight CUDA forward
(AUDIO_TOWER_CUDA_OK, out=(n_frames, 6144) bf16 finite) and a
reference-math allclose(atol=1e-5) check of the codebook sum.
* inkling_video_utils_test.py -- 12 passed, incl. a port of SGLang's
test_video_utils.py::test_sample_video_frames_lengths (same 4 cases and
the same expected frame indices) and a real-weight multi-frame CUDA
forward (VIDEO_TOWER_CUDA_OK, out=(total_patches, 6144)).
* inkling_audio_e2e_test.py / inkling_video_e2e_test.py -- strict TP=4
end-to-end smokes: every prompt must be finite, non-empty and
non-collapsed. Green both baseline (5/5) and with cuda_graph+overlap
enabled (5/5).
Scope note: these prove the modalities run and are shape/dtype/finiteness
correct; they are not a cross-stack numerical parity check against SGLang.
Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
`logits_rows` is a view of the same slice that is written back, so the assignment self-overlaps and torch rejects it with "... refer to a single memory location. Please clone()...". The processors edit in place, so the write-back is redundant anyway; cloning the source makes it overlap-safe. Only reached for requests carrying a py_logits_post_processor, so normal requests are unaffected. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
… parity runs evaluate/interface.py gains summarize_generation_stats/log_generation_stats, and lm_eval emits a greppable GEN_STATS marker per batch. Without it a runaway / no-EOS regression hides behind a parseable answer buried in a wall of repeated text -- the failure mode that cost several bring-up iterations. The MMMU harness/runner changes carry the sharded union runs used for the TRT-vs-SGLang comparison (shard plan, incremental atomic per-item writes so a wall-killed shard keeps everything it scored, cap/max_seq plumbing), plus unit coverage of the answer parser. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Adds streaming-vs-batch equivalence over arbitrary split points, tool-call and repetition segmentation, and end-tokens split across deltas. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Standalone probes used to localize the TRT-vs-SGLang vision divergence: prefill logits, transformers-reference decode, termination behaviour, and a per-layer activation dump. Diagnostics only -- not part of any test suite. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Two in-progress pieces, neither reached by a runtime path yet. MTP static tier: InklingMTPConfig carries the checkpoint's num_nextn_predict_layers / chain_hidden_post_norm / local_layer_ids plus the per-depth banded-attention geometry injected from the text tower, so draft depths in local_layer_ids get SWA head geometry and the rest stay global. The weight mapper gains inkling_expected_mtp_keys() and accounts model.mtp.* as consumed rather than deferred when an mtp_config is supplied; the default mtp_config=None leaves the text-tower accounting byte-identical. Unit coverage for config parse, weight accounting, the BF16/unquantized requirement, and per-depth banding against the checkpoint shapes. MMMU harness: INKLING_MMMU_TEXT_ONLY reruns the same items as pure text (image placeholder stripped, no image attached) so the shared decoder is exercised at the identical bs / cap / overlap regime without the vision path. Default-off — the vision scoring path is unchanged when unset. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…del code The cuda-graph decode collapse is a defect in symmetric all-reduce: a captured NCCL_SYMMETRIC reduce whose send buffer is unregistered while its recv buffer is a registered NCCL window corrupts the run at a 12288 B message. Inkling hits that size exactly -- hidden 6144, bf16, one decode token -- so the first global-attention layer goes non-finite and decode collapses to a repeated token 0. Revert the shared-code mitigation in distributed/ops.py: that file is now byte-identical to its pre-Inkling state, so no other model's all-reduce path changes on our account. Mitigate in modeling_inkling.py instead. The all-reduces that trigger this are built by generic modules -- attention o_proj, MoE down_proj -- so the strategy cannot be passed at construction without editing shared code; rebuilding each AllReduce after super().__init__() keeps the mitigation model-local. Each rebuilt instance carries the module's own mapping and dtype over, so strategy is the only delta. Pinning ONESHOT also drops the window requirement, since AllReduce only takes an NCCL window under NCCL_SYMMETRIC/NCCL/AUTO -- two of the five trigger conditions go away, not just one. Active by default; INKLING_ALLREDUCE_STRATEGY=AUTO restores stock behaviour, and the defect with it, for A/B runs. Measured on job 5728192, alternating arms in one job against one binary: default 0/3 collapse, AUTO 3/3, 331 modules swapped. Cost: symmetric is disabled on every Inkling all-reduce, eager included, and roughly a third of captured decode all-reduces pick it today. The performance impact is unmeasured and should be measured before this is treated as final. This is containment, not a root-cause fix -- the defect remains for any other model that meets all five conditions. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Removes the bring-up's debug scaffolding and completes the deliverables the Inkling NVFP4 change was missing. No model behaviour changes. Deleted 41 debug/localization test files (divergence probes, dump/isolate helpers, teacher-forcing and graph-capture localizers) that existed to find the bring-up's defects and have no role now that it works. Nothing imports them: every deleted basename was grepped across the tree, zero references remain. The two environment variables the model still reads are the ones worth keeping — INKLING_ALLREDUCE_STRATEGY (the escape hatch for the ONESHOT all-reduce mitigation) and INKLING_MOE_BACKEND (the trtllm-gen MoE kernel select) — and both are documented; the 19 debug-only INKLING_* knobs are gone. Completed the deliverables: - docs/source/models/supported-models.md — architecture row, multimodal feature-matrix row, and footnote [^14] covering modality coverage, the unsupported set (MTP, LoRA, function calling, constrained decoding, EPD, mm-hash caching), and the all-reduce mitigation plus its escape hatch. - TestInkling_NVFP4::test_nvfp4 added to the shared multimodal accuracy file rather than a standalone per-model test, with a sourced MMMU reference. - Registered that id in test-db/l0_b200.yml and qa/llm_function_core.txt. - Dropped the orphaned inkling_vision_tower_artifact.json; the surviving regression test consumes the generated artifact instead. Static checks: git diff --check clean, every modified Python file compiles, no stale references to the deleted files. Runtime coverage as of the last completed suite: unit tiers green (vision 43, audio 10, video 12, text 31, collect 95 with zero residual debug files) and GSM8K cg0ov0 parity at delta=0.0. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
The MTP / next-N draft work never reached a runtime path: nothing builds the draft layers and nothing consumes them. Only the config plumbing and the load-accounting existed, so remove them rather than ship dead code. - configs/inkling.py: drop InklingMTPConfig, InklingConfig.has_mtp and _as_mtp_config. mtp_config goes back to a plain retained blob, so the checkpoint still round-trips. - inkling_weight_mapper.py: drop inkling_expected_mtp_keys and the consumed_mtp bucket; inkling_account_checkpoint loses its mtp_config parameter. - test_modeling_inkling.py: drop the four Stage-9 MTP tests. - supported-models.md: the footnote no longer claims the draft weights are weight-accounted. Checkpoint accounting is unaffected: "model.mtp." stays in INKLING_DEFERRED_PREFIXES, so the draft weights are classified as deferred exactly like the audio and vision blocks, and `unaccounted` stays empty. Verified: every touched file compiles, and no reference to any removed symbol remains anywhere in the tree. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…ences MMMU already had a reference entry; the two text benchmarks the bring-up actually measured did not, so the numbers lived only in run logs. Both references are the cached SGLang NVFP4 measurement under the same harness, matching how the MMMU entry was sourced: - GSM8K 95.53 (full set, 0.9553). TRT-Inkling was validated against it with the paired 5x100 protocol: flexible-parse mean 0.968, and all four cuda-graph x overlap-scheduler corners scored 0.98-0.99 on the 100 paired items where SGLang scored 0.99. - MMLU 85.66 (full Hendrycks, 14042 samples, weighted_accuracy 85.6573). The TRT side was measured with the 5-seed text-regression protocol on the harness' 114-item subset (83.33 / 84.21 / 85.09 / 85.96 / 87.72, mean ~85.3). The comment says so explicitly: it tracks the reference, but no full-set TRT-LLM MMLU run has been recorded yet. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…that matters
The bring-up left 22 Inkling files under tests/unittest/_torch/modeling. Most
were per-defect regressions or SGLang-alignment scaffolding whose outcome is
now covered end-to-end by the MMMU/GSM8K accuracy tests. Keep one test per
thing that nothing else covers, drop the rest.
Kept:
- test_modeling_inkling.py config parse, layer classification, weight
accounting over the real checkpoint
- inkling_vision_tower_test.py vision tower
- inkling_audio_tower_test.py audio tower (no accuracy benchmark covers it)
- inkling_video_utils_test.py video utils (no accuracy benchmark covers it)
- inkling_input_processor_test.py multimodal input processor
- inkling_moe_backend_select_test.py the INKLING_MOE_BACKEND kernel select
inkling_mmmu_real_align_test.py was doing double duty: the vision-tower and
input-processor tests import its MMMU item fetch/cache and its importlib
loader. Split that half out as inkling_mmmu_fixtures.py (a fixture module, no
tests) and drop the SGLang-alignment machinery with the rest.
Deleted (16): the mmmu align/harness/run/parser set, image_prompts, the three
per-modality e2e smokes, image_fusion, image_norm_fix, attn_decode_meta,
gate_up_deinterleave, kv_manager_v2, generation_parity and
source_logit_replay.
Every kept file compiles and no reference to a deleted module remains in the
tree.
Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
The multimodal file already carried TestInkling_NVFP4::test_nvfp4 for MMMU, so the vision path had an integration test but the shared text decoder -- what GSM8K and MMLU actually exercise -- had none. Adds TestInkling_NVFP4 to test_llm_api_pytorch.py running both text benchmarks against the references recorded earlier. It mirrors the multimodal class: NVFP4 assert, 16384-token budget for the long chain of thought, and extract_inkling_content as the post-processor so the <|content_thinking|> channel is dropped and only the visible answer is scored. Registered the new id in both lists that already carry the multimodal one: test-db/l0_b200.yml and qa/llm_function_core.txt. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…keep CUTLASS The Inkling routed experts now run only the default CUTLASS MoE backend. The trtllm-gen blockScaleMoe path was an opt-in experiment behind INKLING_MOE_BACKEND=TRTLLM, never the default, and it needed a dedicated routing enum plumbed all the way into the CUDA runner to work. Restored to their pre-bring-up state (the additions were Inkling-only, so these files are now byte-identical to 44f0521): - cpp/.../trtllmGenKernels/blockScaleMoe/runner.h the InklingSinkRenorm = 9 enum value and its name case - cpp/.../trtllmGenKernels/blockScaleMoe/runner.cu the precomputed-routing dispatch branch - _torch/modules/fused_moe/routing.py the matching Python enum value and its autotuner-dummy mapping Removed from the model: - _inkling_trtllm_moe_backend / _moe_config_with_trtllm_backend and the frozen-config copy they needed to retarget moe_backend - InklingMoeRoutingMethod._trtllm_backend, so routing_method_type is always Unspecified and requires_separated_routing goes back to the default - the per-layer INKLING_MOE_SELECT backend-introspection log Also deletes inkling_moe_backend_select_test.py, which existed only to cover that knob, and the stale comment that pointed at it as the fix for the fused combine's cross-row non-determinism. INKLING_ALLREDUCE_STRATEGY is now the only environment variable the model reads. Everything compiles and git diff --check is clean. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
The MMLU comment added in 5f28628 claimed no full-set TRT-LLM run had been recorded. That is wrong. One exists: 2026-07-20, all 14042 Hendrycks samples, weighted_accuracy 82.2176 -- 3.44 points below the 85.66 reference, and it failed the bring-up's 2-point gate. The gap was later traced to an fp4 expert-GEMM family difference (CUTLASS/trtllm-gen vs SGLang's flashinfer) plus non-terminating generation on hard prompts. What is true is narrower: the full set has not been re-measured since those fixes. Post-fix MMLU evidence is subset-only -- a 570-item stratified canary at 84.21 (-1.45, inside the gate) and a 5-seed 114-item regression averaging ~85.3. The comment now says exactly that, including that meeting 85.66 at full scale is unverified. GSM8K's comment was not wrong but was too vague about coverage. TRT-LLM has never been measured on the full 1319-item set: the evidence is the paired 5x100 protocol (flexible mean 0.968), the four cuda-graph x overlap corners at 0.98-0.99 on 100 paired items, and one early full-set attempt that completed only its first 120-item chunk (0.925 vs 0.975). Spelled out. Both files still record the SGLang reference as the accuracy value, so TestInkling_NVFP4::test_nvfp4 grades against a bar the model has not been shown to clear at full scale. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…timodal
The module already carried the image, audio and video paths, so the "vision"
name no longer described it. Rename it and reorganize the contents into four
labelled sections -- vision tower, audio tower, video tower, and the shared
multimodal input processor -- so each modality is easy to locate.
Behavior-preserving. Along the way:
* factor the duplicated tower weight-loading into ``_load_tower_weights``
and the duplicated RMSNorm into a single shared ``InklingRMSNorm``
(was ``InklingVisionRMSNorm``, used by both towers);
* factor the input processor's repeated media-list coercion and its
placeholder/feature-row count checks into small helpers, dropping a
tautological per-item check (``num_tokens`` is built from ``num_patches``);
* trim the docstrings to what the code needs, dropping the development-time
stage/goal references and reference-implementation file paths.
Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Bring-up leftovers that do not belong in the production model:
* cuda_graph_runner: drop the "[cuda-graph] CAPTURED/REPLAYED" evidence
logging and its once-only gate flag. This was pure instrumentation added
to prove the runtime really captured and replayed a graph, and it sat in
shared (non-Inkling) code.
* modeling_inkling: drop the INKLING_ALLREDUCE_STRATEGY environment knob.
The ONESHOT pin is a correctness mitigation, not a tuning option, so it
is applied unconditionally instead of via an A/B toggle (and the log line
that reported it is gone).
* modeling_inkling: drop the explicit-window short-conv decode path
(InklingShortConv.forward_decode, the decoder layer's third branch, and
the attention's conv_states/return_conv_state arguments). Only the
bring-up replay harness ever passed those; the runtime always drives the
short convs through the per-request state pool.
* modeling_inkling: drop the attention's decode_seq_lens / decode_page_table
/ skip_kv_write arguments. The runtime publishes decode metadata into the
layer's stable GPU buffers before capture, so the pre-supplied static
tensors had no caller left; the eager fallback that builds them from the
host block table stays.
* modeling_inkling: drop InklingConvStateCache.reset (unused) and merge
InklingConvRuntime.from_metadata into build (the split existed only so the
replay harness could publish slots itself).
Also rewrite the comments and docstrings across the Inkling _torch files to
describe the code as it stands: no development stage/goal numbers, job ids,
absolute paths into local reference checkouts, or references to the deleted
replay harness. The stale "audio / vision / MTP are deferred" module docstring
now reflects that only MTP is unimplemented.
Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
The Inkling unit tests were five files carrying a bring-up harness rather than a test suite: they loaded the SGLang reference from an absolute path in a local checkout, downloaded MMMU rows over the network into a gitignored cache, wrote JSON artifacts, hardcoded a personal /lustre checkpoint path, and shipped __main__ runners that printed comparison tables. None of that can run in CI. Fold everything multimodal into test_modeling_inkling_multimodal.py -- one file covering the vision, audio and video paths plus the shared input processor -- with small synthetic configs and inputs: no checkpoint, no GPU, no network, no SGLang import, no artifacts. 27 tests, well under a second. Slim test_modeling_inkling.py the same way. The config and layer-classification tests now build their config explicitly instead of requiring the checkpoint, so they actually run; the weight-accounting and tensor-shape tests keep the checkpoint (index JSON only, no weights) and resolve it through the standard llm_models_root() with an INKLING_CHECKPOINT override, so they skip cleanly instead of always skipping on a path that only existed on one machine. Removed: inkling_vision_tower_test.py, inkling_input_processor_test.py, inkling_audio_tower_test.py, inkling_video_utils_test.py (folded in) inkling_mmmu_fixtures.py (MMMU downloader + SGLang loader; no longer used) Also drop the .gitignore entries for the deleted caches and artifacts. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…nkling PR Run the repo's pre-commit formatters (isort, yapf, ruff, ruff-format) over the files this PR touches, and fix the eleven ruff-legacy D205/D209 docstring regressions it flags in tensorrt_llm/evaluate/interface.py and tests/unittest/llmapi/test_reasoning_parser.py. Formatting and docstring wording only; no behavior change. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Two bring-up leftovers outside _torch:
* evaluate: drop summarize_generation_stats / log_generation_stats and both
call sites (Evaluator.evaluate and LmEvalWrapper). This logged a greppable
GEN_STATS line with the finish_reason mix and generated-token distribution
so a runaway/no-EOS regression would be visible while bringing the model up.
It is observability scaffolding, not part of the eval contract: it never
influenced what was scored, nothing consumed the marker, and no test
covered it.
* docs: drop "The text decoder is also usable standalone (text-only) via the
InklingForCausalLM architecture" from the Inkling footnote. It is not true.
InklingForCausalLM carries no @register_auto_model, so it is not in
MODEL_CLASS_MAPPING and no checkpoint can select it; the config registry has
no inkling_text entry either. The class is the base that
InklingForConditionalGeneration derives from, and the inkling_text handling
in config_utils/_util exists for that nested sub-config, not for standalone
loading. The published checkpoint declares InklingForConditionalGeneration
and the text-only accuracy test loads it through that same architecture, so
registering the class would advertise a path with no checkpoint to exercise
it and no test coverage.
Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
All three references were previously the SGLang measurement. GSM8K and MMLU carried a long note that TRT-LLM had never been confirmed at full scale, and MMMU claimed TRT-LLM matched SGLang item-for-item, which is not what the data shows. GSM8K and MMLU have now been run on the complete datasets, TRT-LLM and SGLang side by side, at TP=4 with CUDA graph and the overlap scheduler on, batch 8, driven by the same client so prompt rendering and scoring are shared code. MMMU was already measured on the full 858-item pool during the bring-up: GSM8K TRT-LLM 96.13 (1268/1319) SGLang 96.59 (1274/1319) -0.45 pt MMLU TRT-LLM 85.35 (11985/14042) SGLang 85.48 (12003/14042) -0.13 pt MMMU TRT-LLM 79.72 (684/858) SGLang 78.32 (672/858) +1.40 pt The references now hold the TRT-LLM numbers, which is what CI reproduces, and the comments record the SGLang cross-check. The earlier -3.44 pt MMLU gap is gone now that the routed experts run on CUTLASS only and the non-terminating generations are fixed. The MMMU comment also drops the item-for-item claim and records what the pool actually shows: the two stacks agree on aggregate accuracy but 74/858 (8.6%) of items score differently in both directions, which is inherent to two different NVFP4 implementations resolving near-ties differently deep in a long-CoT decode. Aggregate accuracy, not per-item identity, is the metric to gate on. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Brings the branch up to 93e7c91 (402 upstream commits since 44f0521). Six files conflicted; all resolutions keep both sides: * supported-models.md upstream took footnote [^14] for MiniCPM-V, so the Inkling note moves to [^15] (3 table rows renumbered) * _util.py union of the import blocks; the new is_hybrid_linear V2 guard sits alongside the Inkling one * model_engine.py upstream's steady-gen fast path and the Inkling runtime publish are independent additions; upstream also fixed the logits self-assignment defect by dropping the redundant write-back, which supersedes the clone() this branch carried * py_executor_creator union of the config_utils imports * lm_eval.py six additive-kwarg conflicts, union throughout * test_lm_eval.py both sides appended an independent test section One resolution is not textual. Upstream's new _apply_steady_gen_fast_prepare returns early from _prepare_tp_inputs, bypassing the tail where Inkling publishes its short-conv pool rows and per-layer attention decode metadata. git reports no conflict there -- the two sides touch different lines -- but an unchanged generation-only batch is exactly Inkling's steady-state decode, so a plain merge would have left the captured graph reading capture-time page tables. The fast path now publishes before returning; it is a no-op for models without a registered CONV_STATE_MANAGER. Also initializes the new 3rdparty/MSA submodule (upstream NVIDIA#16291), which setup.py now requires. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
…d it Two review findings on InklingDecodeMeta, both on the decode critical path. **Stale publication.** `ready` was a plain flag that only `refresh` cleared, so it described "some forward published these buffers", not "this forward did". A forward that reached the decode kernel without the engine having prepared -- an eager fallback, or a prepare path that returns early -- read the previous step's seq_lens and page table. The same requests advance num_cached_tokens_per_seq every step, so a page table one step out of date silently drops a newly allocated page. Request ids and the batch size cannot detect that. `prepare_inkling_attn_decode` now bumps a shared per-forward counter before doing anything else, and `ready` is a property comparing the layer's recorded epoch against it. A layer that is not refreshed this forward reports False by construction, so the attention takes its host fallback instead of reading stale buffers. This is the same contract the upstream merge broke once already (a new fast path bypassed the publish site); making the invariant self-checking means the next such refactor fails loudly rather than silently. **Per-layer work on every token.** `refresh` ran for all 66 layers per decode step, and each call allocated two pinned host tensors and built the page table with one `torch.tensor` per row -- 132 cudaHostAllocs and 66*num_gen tensor constructions per token. The total-KV lengths do not depend on the layer, so they are now built once per step and shared; the per-layer pinned page-table staging is allocated once and reused, and filled through its numpy view with a vectorized row assignment. Behavior is unchanged when the engine publishes every forward, which is the runtime path. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Three review findings on the way a checkpoint is read. The weight mapper resolved its geometry from whatever config it was handed. ``ModelLoader.load`` initializes it with the TOP-LEVEL ``InklingConfig``, which keeps the decoder geometry under ``text_config`` and has no ``vocab_size`` or ``n_routed_experts`` of its own, so ``preprocess_weights`` raised ``AttributeError`` on the unpadded-vocab lookup and ``_map_expert`` silently fell back to ``tensor.shape[0]`` for the expert count. The mapper now resolves ``text_config`` itself, which covers both entry points: the loader-supplied mapper and the one ``InklingForConditionalGeneration.load_weights`` builds. ``InklingTextConfig`` unconditionally set ``moe_intermediate_size`` from ``intermediate_size``, discarding the value a ``config.json`` had already stored through ``**kwargs``. It now reads the kwarg back first, matching how ``max_position_embeddings`` is handled two lines above. ``load_modelopt_quant_config`` gained ``_algo_or_none`` to map both JSON null and the strings "none"/"null" onto ``None``, but the MIXED_PRECISION branch still built ``QuantAlgo`` directly, so a mixed-precision checkpoint spelling ``kv_cache_quant_algo: "none"`` would still raise. That branch uses the helper now. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Each of these turned a bad input into wrong output rather than an error. The Triton kernels map a query head to its KV head as ``cur_head // kv_group_num``. Both entry points computed ``kv_group_num = num_heads // num_kv_heads`` without checking divisibility, so a non-divisible pair produced a wrong mapping inside the kernel with no diagnostic. Both now assert. The prefill entry also unpacked ``total_tokens`` without using it (ruff RUF059); renamed to ``_total_tokens``. ``InklingInputProcessor.assemble`` dispatches on the token value, and both placeholder ids are config-overridable. If a config gave them the same value every occurrence was consumed as an image, the audio branch never ran, and the placeholder-count check reported the mismatch against the wrong modality. The constructor now rejects the collision. ``InklingReasoningParser.finish`` cleared ``_buffer`` but left ``_kind`` at the last block kind, so reusing an instance for a second stream would route that stream's first segment by the previous stream's channel. ``parse`` already resets both. Not reachable today (the factory builds one parser per request), but the asymmetry is the kind that survives a refactor. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
The supported-models footnote named GSM8K and MMMU as the accuracy gates, but TestInkling_NVFP4 also evaluates MMLU on the text decoder. Name both. The MMMU test docstring claimed Inkling was validated against SGLang "item-for-item". It is not: 74 of the 858 pool items score differently, in both directions, and the harness gates the aggregate score. Say what is actually validated, and why per-item identity is the wrong metric here -- two different NVFP4 implementations resolve near-ties differently deep in a long-CoT decode. Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Signed-off-by: KleinBlueC <102355066+KleinBlueC@users.noreply.github.com>
Signed-off-by: kleinc <kleinc@nvidia.com>
The conv pool is cross-step per-request GPU state owned by InklingHybridCacheManager and published from InklingAttentionMetadata.prepare, i.e. runtime state, not model weights. Living in modeling_inkling.py forced attention_backend/inkling/cache_manager.py to import models/modeling_inkling from inside two functions to dodge the import cycle that a top-level import would close. Moving InklingConvState/Cache/Runtime and the short-conv helper into attention_backend/inkling/conv_state.py removes the reverse dependency; the imports are now plain top-level ones. Pure relocation: no behaviour change. _apply_sconv is renamed apply_short_conv because it is now a cross-module entry point. Signed-off-by: kleinc <kleinc@nvidia.com>
…l layer ATTENTION_DEVELOPER_GUIDE.md §1.1 puts KV write, page-table construction and prefill/decode dispatch in the backend and leaves projections, extra Q/K processing and the output projection in the module. Inkling had the first group in modeling_inkling.py, which is why InklingTritonAttention was an empty class carrying only the Metadata attribute. _attention/_run_context/_run_generation move to InklingTritonAttention.forward_inkling, following MiniMax-M3's forward_sparse (sparse/minimax_m3/triton_backend.py): a name-explicit backend entry point, because rel_logits has no home in the shared AttentionForwardArgs and adding a field there would edit a dataclass shared by every other model for one model's sake. The module keeps _project/_build_rel_logits/o_proj. Pure relocation. The three per-layer scalars the moved code reads (sm_scale, rel_extent, window_left) are assigned onto the backend in InklingAttention because create_attention() has a fixed kwarg list; layer_idx the backend already carries. Two docstrings claimed the backend existed so the base module could assign local_layer_idx -- it never did; the moved code uses self.layer_idx and KVCacheManagerV2 maps it through layer_offsets itself. Signed-off-by: kleinc <kleinc@nvidia.com>
… ceiling test_expert_parallel_ceiling_says_it_is_measured_not_theoretical has been failing on this branch since e163a57 condensed the comment above the guard: the condensing dropped the GSM8K numbers, the SIGSEGV observation and the fact that changing max_batch_size / max_num_tokens does not move the crash. The test asserts on those words precisely because they are an experiment log, not prose -- without them the next reader cannot tell whether the ceiling is a measured limit that can be raised or a property of the design. Restores the three-line result table and the ruled-out cause. No behaviour change; the guard's conditions and messages are untouched. Found while validating the attention layering refactor and unrelated to it. Signed-off-by: kleinc <kleinc@nvidia.com>
The compute moved into InklingTritonAttention.forward_inkling, which reads sm_scale / rel_extent / window_left off the backend -- values create_attention() has no kwarg for, so the module assigns them after construction. Nothing tested that hand-off, and it is the one part of the relocation that fails silently: a local layer left with a global layer's window_left=-1 attends outside its sliding window and produces plausible, wrong text rather than an error. Checks both layer families against the module's own values, and asserts the two families actually differ so the comparison is not vacuous. Signed-off-by: kleinc <kleinc@nvidia.com>
…y on Review feedback on InklingHybridCacheManager asked whether subclassing KVCacheManagerV2 is the right shape. Subclassing is not the deviation -- MiniMaxM3KVCacheManagerV2, DeepseekV4CacheManager, DSACacheManagerV2 and MambaHybridCacheManagerV2 all do it, and Inkling structurally requires V2 for its per-layer KV-head split. What deviated is the short-conv state pool: a private torch allocation that grew on demand, outside every V2 hook, with slot arithmetic that ignored pipeline parallelism and attention DP. Growth was the defect. _grow reallocated every layer buffer and state_indices, and write_state_indices refused it only while a graph forward was running. Growth in an eager step AFTER capture was allowed, and a captured graph then holds pointers into freed allocations -- which does not raise, it reads whatever the caching allocator handed out next. The pool is now fixed at construction: max_batch_size * pp_size request rows -- the count MambaHybridCacheManagerV2 calls _max_resident_sequences -- plus one permanently reserved row shared by every CUDA-graph padding sentinel, plus one attention-DP idle-dummy row when that is enabled. This is MambaCacheManager's _padding_slot / _attention_dp_dummy_slot layout, and it fixes a second problem on the way: cuda_graph_runner keeps one sentinel id per runtime draft length, and charging each of them a real row silently shrank the servable batch. Reserved rows never re-enter the free list, so a real request cannot be handed a row the next padding batch overwrites. Exhaustion raises and names the cause instead of quietly aliasing live state. Fixing the size also repairs the memory accounting. The pool is invisible to V2's byte quota and is counted only because the throwaway estimation manager holds one while configure_kv_cache_capacity reads peak memory -- which is correct exactly when the estimation pool and the serving pool are the same size. The manager now logs the pool's row count and MiB so the term is visible. Two smaller fixes in the same file. The conv dtype is resolved rather than guessed: a torch.dtype or HuggingFace's string spelling of one, else a ValueError naming torch_dtype, where the previous silent bfloat16 fallback would have built an fp16 release's pool in the wrong dtype. And pretrained_config / mapping / max_batch_size are declared parameters instead of kwargs subscriptions, so omitting one is a TypeError naming it. Finally, the per-forward context/generation split moves to InklingAttentionMetadata, which now asks the manager for the pool (conv_state_cache, plus get_conv_states / get_state_indices named after BaseMambaCacheManager's hooks) and builds InklingConvRuntime itself. The manager owns the pool's lifetime; splitting a batch is metadata work. Validated on Inkling-small-NVFP4, TP=4/EP=2: unit tests 73 passed / 4 skipped; token-exact A/B against the pre-change tree VERDICT=IDENTICAL with and without CUDA graphs; sampled GSM8K (0 regressed / 1 improved of 83) and MMLU (asymmetry -2 of 400) aligned with the recorded baselines. Signed-off-by: kleinc <kleinc@nvidia.com>
Inkling's four short convolutions per layer hold a kernel_size - 1 window of per-request state in InklingConvStateCache -- a plain torch pool with no page table and no entry in the disagg page-table builder. A cache transfer therefore moves the paged KV correctly and leaves the conv windows behind, and the generation instance resumes convolving against zeros: wrong output, no error. The C++ transceiver route is already refused for every KVCacheManagerV2 in _util.py, so this was only reachable through the Python one (KvCacheTransceiverV2, transceiver_runtime="PYTHON"). The refusal goes beside the block-reuse and chunked-prefill refusals in reject_unsupported_inkling_kv_cache_features, which is the one place that sees the resolved kv_cache_config alongside llm_args, and states the reason so a reader does not conclude the guard was over-cautious. No-op for aggregate deployments: the guard is off unless cache_transceiver_config declares a backend. Signed-off-by: kleinc <kleinc@nvidia.com>
One conflict, in tests/integration/defs/accuracy/test_llm_api_pytorch.py: both sides added an import at the same line -- this branch's extract_inkling_content and main's pad_up. Both are used in the merged file, so both imports are kept. Nothing else in main's delta since d9c8974 touches this branch's areas: it is VisualGen (Hunyuan Video 1.5), mpi_session, and a flashinfer workspace test. pyexecutor, attention_backend and kv_cache_manager_v2 are untouched by main. Signed-off-by: kleinc <kleinc@nvidia.com>
CI runs yapf and ruff-format over the PR's changed files and fails the job when a hook rewrites one. Three files were left in a shape the hooks do not agree with, so every push failed on formatting alone: inkling/conv_state.py yapf joins the parenthesised conditional pyexecutor/_util.py yapf re-wraps the enable_cache_transceiver args pyexecutor/config_utils.py the keyword-only signature re-indents This applies exactly what the hooks produce -- the diff is byte-identical to the one CI printed -- and re-running the full hook set over the changed files now converges with nothing modified. No behaviour change. Signed-off-by: kleinc <kleinc@nvidia.com>
…case Adding the disaggregated-serving refusal left the paragraph below it still saying "Both leave a context request with history", which is now both wrong (there are three) and wrong about disagg (it does not involve cached history on a context request at all). Say which two share that failure mode and how the third differs. Signed-off-by: kleinc <kleinc@nvidia.com>
InklingForConditionalGeneration built both media towers eagerly in __init__:
self.visual = InklingVisionModel(vision_config).to(torch.bfloat16)
Under MetaInitMode the .to() dispatches aten._to_copy.default on meta
tensors, which that mode rejects with MetaInitException. model_loader wraps
the mode around the ENTIRE AutoModelForCausalLM.from_config call, so one
tower raising there cost the whole text stack its meta init and the model
was rebuilt with real allocations -- exactly what meta init exists to
prevent ("skip memory allocation when creating weights, which avoids OOM
when GPU memory is not enough for all weights"). Every Inkling checkpoint
carrying a vision_config hit this on every load, and it was visible only as
an INFO line, 'Fallback to regular model init'.
Build both towers through _build_replicated_bf16_tower, which returns the
config instead of the tower when meta tensors are present, and rebuild in
load_weights before loading into them. Same shape as modeling_kimi_k3_vl
and modeling_kimi_k25, which defer their vision encoders for this reason;
this one also checks _has_meta_tensors, because construction can succeed
and leave meta parameters behind for the .to() to trip over later.
No numerical change: this only moves when the towers are allocated, and
their weights are loaded from the checkpoint either way.
Signed-off-by: kleinc <kleinc@nvidia.com>
Signed-off-by: kleinc <kleinc@nvidia.com>
…nv state Inkling had hand-rolled four things the framework already provides. This moves each onto the existing hook and deletes the local copy. 1. **Short-conv state is now declared to KVCacheManagerV2 as SSM layers.** `SsmLayerConfig` is the framework's container for per-request fixed-size state -- `MambaHybridCacheManagerV2` already carries Mamba's conv/SSM state through it -- and it fits a short-conv window for the same reason: the window is `[channels, kernel-1]` per request and does not grow with the sequence, so it cannot be an attention buffer (`BufferConfig.size` there means bytes *per block*). The layers are appended after the ones `_build_base_config` produced rather than interleaved, which keeps every attention `layer_id` where it was, so `get_buffers` / `get_batch_cache_indices` / the page tables are untouched. The point is not the line count -- it is that the pool's bytes now sit inside V2's quota. They used to be a plain `torch` allocation no quota knew about, accounted for only because the throwaway manager built during KV-cache size estimation also held one, so `configure_kv_cache_capacity`'s peak-memory reading happened to include it. That was correct only while the estimation pool and the serving pool were exactly the same size, an invariant nothing enforced. V2 owns the bytes; `InklingConvStateCache` still owns the request-to-row mapping, matching `MambaHybridCacheManagerV2`. Overriding `_get_pool_roles` is required: the base returns `Role.KEY` unconditionally and the conv-only pools hold no KEY, and `commit_min_snapshot=True` is asserted by `KVCacheManagerConfig` whenever an SSM layer is present. 2. **The backend implements the standard `AttentionBackend.forward`.** The two inputs that are Inkling's alone -- the per-(query, head, distance) additive bias and the mixed-batch certificate -- ride `AttentionForwardArgs.sparse_backend_args`, which is the registered slot for exactly this. `forward_inkling` is gone. The shared `relative_attention_bias` field cannot carry the bias: it is T5's `[num_heads, num_buckets]` table broadcast across the batch, while Inkling's is projected from the hidden states per query token, and passing it there also forces the unfused C++ path that materializes the full score matrix. 3. **Per-step metadata reuses the base fields.** `ink_seq_lens` is deleted in favour of a slice of `TrtllmAttentionMetadata.kv_lens_cuda`, which the base already fills from the same re-clamped `num_cached_tokens_per_seq` into a buffer it already keeps CUDA-graph-stable (note: `kv_lens_cuda`, not the host `kv_lens`, which adds `num_extra_kv_tokens`). The page table cannot be borrowed the same way -- the base's is laid out for the C++ attention op -- but it is now keyed by page group rather than by layer: `get_batch_cache_indices` resolves a layer to `(pool_id, index_scale)` and depends on nothing else, and Inkling has two KV geometries across 66 layers, so this drops ~33x of per-step host work in a host-bound phase. 4. **The package moved to `attention_backend/sparse/inkling/`.** Inkling is not a sparse-attention algorithm, and the docstrings say so in as many words: what unifies `sparse/`'s members is that each needs its own backend + metadata + cache manager, which is Inkling's shape exactly. It was the only such package outside that directory. `get_attention_backend` also stops carrying a model-specific branch: reserved names resolve through a lazily-imported registry, so adding a model no longer edits the family selector. The selection still keys on the NAME, deliberately. Routing it through `sparse/registry.py` instead was tried and reverted: that registry dispatches on `SparseParams`, which only the `Attention` module has, while `PyTorchModelEngine`, the KV-cache creator and a dozen vision encoders pass a bare string and read `.Metadata` off the result. Under that arrangement the engine resolved `TrtllmAttentionMetadata`, the decode seq lens and page table were never published, and warmup's first mixed batch died inside the backend. The name is Inkling's only identity outside the module layer. Verified against small-model full sets, each judged against a same-code control rather than a fixed tolerance band -- MMMU's recorded +/-0.7 pt band is narrower than one standard error at n=857 and flags unmodified code as a regression: | set | n | this | control | delta | |-------|-------|-------|---------|--------| | GSM8K | 1319 | 96.29 | 96.36 | -0.08 | | MMLU | 14042 | 79.38 | 79.45 | -0.07 | | MMMU | 857 | 76.43 | 76.55 | -0.12 | The MMMU control is a rerun of the pre-refactor tree through the same harness and shards; paired over 857 items the difference is 47/46 flips, McNemar z=0.00. MMMU health flags are clean (image_blind=0, collapsed=0) in both arms. Unit tests: 81 passed. Signed-off-by: kleinc <kleinc@nvidia.com>
Follow-up to the previous commit, which kept a model-specific `elif` in `get_attention_backend` and resolved Inkling by backend NAME. This makes `sparse/registry.py` the single selection path for both the backend and the cache manager, so the family selector carries no model-specific branch at all. What makes that possible: `ModelConfig` is a plain dataclass and `sparse_attention_config` is only *annotated* `Optional[SparseAttentionConfig]`, never validated against the discriminated union in `llmapi/llm_args.py`. So `ModelConfig.from_pretrained` injects a standalone `BaseModel` -- `InklingSparseAttentionConfig` -- keyed on the checkpoint architecture, before the instance is frozen (the same seam DeepSeek-V4 uses to derive its own). The module layer then lowers it to `SparseParams` through the existing code path, and every consumer that holds no `ModelConfig` reaches the same backend because `PyTorchModelEngine` re-reads the field off the built model (`model_engine.py:553`) rather than off `llm_args`. Nothing is added to the user-facing schema: no `attn_backend` enum value, no union member, so no api-stability snapshot change, no golden-manifest regeneration and no telemetry review. Inkling has one correct backend and no alternative to choose between, so a user-settable knob would have one legal value. `get_model_defaults` therefore stops pinning `attn_backend`, and `_assert_inkling_attn_backend` now guards the *family* instead of the name -- the `VANILLA` and `FLASHINFER` registries do not know the `"inkling"` algorithm and would otherwise surface an error naming neither Inkling nor the setting. Two consequences of the field becoming non-`None` for a model that is not sparse, both deliberate: * `get_sparse_attn_kv_cache_manager` needs an `inkling` entry. Without it `get_kv_cache_manager_cls` takes its sparse branch and raises "Unsupported sparse attention algorithm" at engine startup, before the `is_inkling` route in `_non_hybrid_kv_cache_manager_cls` is ever reached. That route is removed as dead code; the `is_inkling` term in the `needs_v2` test stays, because it runs even on the registry path and an explicit `use_kv_cache_manager_v2=False` must not silently mis-size the per-layer pool. * the "Using sparse attention: ..." line in `modules/attention.py` now prints for a dense model. Silencing it would mean a model-specific branch in a file shared by every attention module -- the kind of edit this arrangement exists to remove -- so it is accepted as the visible cost of the field saying something untrue. The standing liability is that any future reader treating `sparse_attention_config` as "this model is sparse" needs the same audit, and its failure mode is a wrong branch rather than an error. The alternative kept in the parent commit is one `elif` that cannot be reached by accident. Both are on the table; this branch exists so the two can be compared as code rather than as description. Behaviour is unchanged -- selection resolves to the same `InklingTritonAttention` / `InklingHybridCacheManager` / `InklingAttentionMetadata` as before, which the tests assert directly. Unit tests: 83 passed. Signed-off-by: kleinc <kleinc@nvidia.com>
…ge table
InklingAttentionMetadata staged its own per-geometry page table every decode
step: a pinned host buffer, a Python loop over requests and blocks, and one
non-blocking H2D per KV geometry -- on the host-bound decode path.
The base metadata already publishes the same information. Every difference from
what the Triton kernels want is an encoding the kernel absorbs:
- plane 0 of kv_cache_block_offsets holds index_scale * base_page_index
(copyBatchBlockOffsetsToDeviceKernel), and get_batch_cache_indices computes
base_page_index * index_scale // kv_factor, so the two differ by exactly one
integer division -- PAGE_DIV, a constexpr power of two, once per KV tile.
- the leading axis is per-pool, which hands back one table per KV geometry
for free; that deduplication used to be a hand-rolled group key.
- absent blocks are already 0, matching what the seq_len mask assumes.
Borrowing also removes an assumption instead of adding one: the staged table
packed valid ids to the front, which only matches the kernel's
k_pos // page_size addressing while no request has an interior hole in its block
list. The borrowed tensor is positional by construction.
Deletes ink_page_table, _ink_pt_host, ink_max_pages, ink_cap, _ink_page_group
and _ink_ensure; keeps a static layer -> row map, which also has to honour
enable_swa_scratch_reuse (rows become per attention-op layer in that mode) and
to check that a layer's page-index scale matches its pool's, since the C++ copy
encodes with the pool-level one.
TLLM_INKLING_PT_CROSSCHECK=1 re-runs the deleted host path per step and asserts
the two tables agree; off by default.
Signed-off-by: kleinc <kleinc@nvidia.com>
… a warning The C++ copy writes the full row width from a host mirror initialised to zeros, so the borrowed row's tail past a request's block count should be 0. Whether that mirror is re-zeroed when a slot is reused by a shorter request is not established, and the decode kernel provably never reads there -- mask_n = k_pos < seq_len bounds every access by the request's own KV length. Asserting on it would put a false-alarm path on the gate whose whole job is to make the borrowed page table trustworthy, so it reports instead. Signed-off-by: kleinc <kleinc@nvidia.com>
…fig interface Executor init died with AttributeError: 'InklingSparseAttentionConfig' object has no attribute 'to_sparse_metadata_params' from model_engine._set_up_attn_metadata, which calls that method unconditionally whenever sparse_attention_config is not None. The config was a bare BaseModel duck-typing BaseSparseAttentionConfig, so it had whichever methods were transcribed and none that the interface grew. A test made this permanent by asserting the config was *not* a subclass of the interface, treating that as the way to stay out of the user-facing schema. Those are separate properties: the SparseAttentionConfig union is explicitly enumerated (llm_args.py:3630-3639) and nothing derives the schema from __subclasses__, so inheriting adds nothing user-facing. Subclass now, and assert only what actually matters -- absence from the union. This is a defect of the parent commit, not of the page-table change; it had never been run end to end. Found by job 6275473. Signed-off-by: kleinc <kleinc@nvidia.com>
…fix its gate
Two defects in the validation harness, both found by job 6275760, which reported
CROSSCHECK FAILED (rc=8) on a run with zero assertion hits.
1. The gate was 'grep -nE ... | head && { fail; }'. A pipeline's status is its
last command's, and head always succeeds, so the failure branch fired
unconditionally. Rewritten as 'if grep -qE'. This is the same family as the
earlier quoting defects: the verification machinery being wrong is worse than
no verification, because it is believed.
2. The crosscheck was silent on success, so a clean log was indistinguishable
from a crosscheck that never executed -- the env var failing to reach an MPI
worker would look identical. It now emits one INKLING_PT_CROSSCHECK ACTIVE
line per process reporting how many layer/request pairs agreed, and the
harness treats the absence of that line as a failure (rc=7) rather than a
pass.
Also adds status.sh: the server logs are dominated by newline-free progress bars,
so a plain tail is tens of kilobytes and buries the four lines that matter.
Signed-off-by: kleinc <kleinc@nvidia.com>
tensorrt_llm.logger.info(*msg) concatenates its arguments; it does not %-format them the way stdlib logging does. The lazy form printed a literal '%d layer/request pairs agreed', losing the counts that are the only reason the line exists -- job 6276424 proved the crosscheck ran but could not say over how much. Signed-off-by: kleinc <kleinc@nvidia.com>
Signed-off-by: kleinc <kleinc@nvidia.com>
…metadata type With both decode buffers borrowed from the base, the per-step counter carried no information of its own. model_engine assigns _num_generations and request_ids from one scheduled batch in one block (model_engine.py:4388-4391), so num_generations already equals len(request_ids) - num_contexts, CUDA-graph padding included -- those rows enter the batch as dummy generation requests. An assert in _prepare_inkling_decode pins that equality. What the counter was really doing lived in the backend's 'getattr(attn_metadata, "ink_num_gen", 0) == num_req': a foreign metadata object -- the P4 failure, where a degraded selection handed back TrtllmAttentionMetadata -- fell out of the fast path on the getattr *default* rather than on a stated check. That is now an explicit isinstance, so swapping in num_generations (which every metadata has) cannot let a foreign object through. The three early returns in _prepare_inkling_decode become plain 'no decode work in this batch'. None is reachable from the backend: no manager means it cannot get_buffers, zero generation rows means it never enters _run_generation, and no owned layers means no attention module runs. Inkling-private metadata state is now three fields, all short-conv: _ink_pt_rows (a static layer -> row map that also carries the per-layer vs pool index_scale check), ink_conv_cache and ink_conv_rt. The attention path contributes none. Unit tests 90 passed / 4 skipped (job 6277501). Signed-off-by: kleinc <kleinc@nvidia.com>
Comment+docstring share of the package was 44%. Cut to 38% (-218 lines) by dropping three kinds of prose: restatements of what the code does, narrative about earlier versions of this work, and facts repeated across files. What is left is the reasoning that cannot be read off the code. Also corrects a stale claim in the package docstring, which still said Inkling was selected by backend name in attention_backend/utils.py and that the registry route had been 'tried and reverted'. This branch *is* the registry route. Behaviour-neutral: comparing HEAD against the working tree as ASTs with every docstring stripped, all seven modules are node-for-node identical. Unit tests 90 passed / 4 skipped (job 6278297). Signed-off-by: kleinc <kleinc@nvidia.com>
Open
1 task done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@coderabbitai summary
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.