Repository navigation
[Fix] kv_cache: Drop fp8 scales when using nvfp4 KV cache - #42419
devtyagi3909 wants to merge 2 commits into
Conversation
|
fixed lint. humanized the log message. |
|
generalised the check per your suggestion to catch both directions (fp4 runtime with fp8 checkpoint scales, and vice versa). also ensured the filtered scales degrade perfectly to dynamic. pushed the update. would be awesome if you could run the full harness on the 2x5060 rig to validate this end-to-end! |
…VFP4 KV cache + Gemma4 text config Skip reversed per user direction: NVFP4 KV (RDXA + NVDA NVFP4 variants of Qwen3.8-Flash-Next) is an ACTIVE FUTURE config. Squash of pr43247 net diff (22 files, base 5b5d721) with toolchain.py excluded (entry 65 already applied; modelopt_quant.py entry-63 hunks auto-merged idempotently via -3). Carries the branch's merged-in PR sgl-project#42419 (drop fp8 scales under nvfp4 KV, still OPEN upstream) as part of the net diff. Conflicts resolved (4): - flashinfer_backend.py x4: UNION our KVLocPlan `plan=` plumbing with the PR's per-wrapper head_dim/num_kv_heads/num_qo_heads kwargs (heterogeneous SWA dims) at all 4 call_begin_forward sites + signature keyword-only marker preserved; PR's custom_kv_indices wiring into update_sliding_window replaces entry 175's single-wrapper RuntimeError guard (PR intent). - memory_pool.py: MHATokenToKVPool.set_kv_buffer_prefix_valid keeps our removed conversion preamble (fp8 fused-scale paths) + takes the PR's is_quantized_kv_cache early-return at its anchor; _safe_zeros Float4 allocation hunks auto-merged. - pool_configurator.py: took the PR's has_heterogeneous restructure (self.kv_cache_dtype_str == kvc.kv_cache_dtype_str, __init__:220) over our equivalent kvc. spelling; _compute_qsa_cell_size tail preserved. - utils/common.py: make_layers empty_cache-per-layer generator body (from d9e1d68) inside our layer_stack/neighbour wrapper. Test adaptation: test_flashinfer_nvfp4_speculative.py __new__ stub gains prefill_wrappers_ragged/prefill_wrapper_ragged (PR adds the attribute read in init_forward_metadata; real __init__ sets both) -- back to 6P. Battery (CVD=9): shipped fp4_kv_cache_quant_method+nvfp4_moe_backend+ spec 31P; pool_configurator 13 subfail = entry-175 env class (is_cuda False, no GPU); server_args/qsa_kv_pool/yarn/ple_staging/kv_loc_plan/ modelopt-nvfp4 set 408P+5skip; 8 imports incl. scheduler OK; ruff repo-ruleset clean; ruff-format drift from the PR fixed in 9 files (verified cosmetic). PRE-EXISTING red re-confirmed on stack tip: test_kernels_namespace reclassified-entry-point inventory (batch C qwen4_short_conv entry not inventoried -- owed, unrelated to this PR).
|
End to end on SM120 this fixes NVFP4 KV: the needle test at 8K to 96K goes from 6/36 to 36/36, level with the default FP8 launch.
With this PR NVFP4 is level with the default FP8 launch on all three measures, and main's BF16 row shows the same breakage. Two gaps against chengye75-star's invariant:
CommandsThe job script, the client scripts, the per-arm outputs and the probe of the filter are in https://gist.github.com/waynehacking8/6081b45e2119aace7844e2d5988a491f. |
…e bf16 runtime (sgl-project#42419) Signed-off-by: DEV TYAGI <devtyagi3909@gmail.com>
21d303b to
a47c58c
Compare
|
thanks for verifying on sm120. rebased on latest main and addressed both gaps: moved the filter inside the mismatch block so fp8 launches preserve checkpoint scales, and added the bf16/unquantized mismatch check so static scales get dropped for bf16 runtimes as well. added unit test coverage in |
This comment has been minimized.
This comment has been minimized.
| if isinstance(kv_scheme, dict): | ||
| checkpoint_bits = kv_scheme.get("num_bits") | ||
| expected_bits = None | ||
| if runtime_kv_dtype: |
There was a problem hiding this comment.
With the default --kv-cache-dtype auto this drops the scales: expected_bits stays None for "auto", but configure_kv_cache_dtype later resolves auto to fp8_e4m3 for compressed-tensors checkpoints with a supported kv_cache_scheme, so the FP8 pool runs with scale 1.0. A probe of the filter shows the scales kept on base eb4b167d27, dropped on this head, and kept again with the change below (bfloat16 still drops them): https://gist.github.com/waynehacking8/0685c4403346f321f0896860aa882f42. A (8, "auto", False) case with kv_cache_quant_algo="FP8" in the new test would cover it.
| if runtime_kv_dtype: | |
| if runtime_kv_dtype == "auto": | |
| # configure_kv_cache_dtype resolves auto to an FP8 pool here. | |
| algo = getattr(quant_config, "kv_cache_quant_algo", None) | |
| if isinstance(algo, str) and algo.upper() == "FP8": | |
| expected_bits = 8 | |
| elif runtime_kv_dtype: |
There was a problem hiding this comment.
i see the catch on the auto resolution for compressed-tensors. applied the suggestion and added the (8, "auto", False) test case with kv_cache_quant_algo="FP8". pushed.
…tensors checkpoints Signed-off-by: DEV TYAGI <devtyagi3909@gmail.com>
…ge damage — entries 61,75,133,137,140,162,176,178,201,247,300,110 LAST B4 COMMIT: 14 markers consumed (correction to B4-3's body: the scheduler/schedule_policy pair was NOT deleted there -- consumed here, so the true session census is 23/23, B4=0 on the tree, foreign B1 15/B2 2/B5 23 INTACT (grep census).) IO_STRUCT 1F VERDICT (the session-1 NEEDS-ATTENTION flag): NOT a fork-delta loss, NOT env -- UPSTREAM'S OWN guard extension vs stale fork stub: _validate_joint_schema_request grew a pointer_head arm in-range (origin/main tokenizer_manager.py:1455, ModelConfig.pointer_head), while our test's SimpleNamespace stub predated it. Fixed test-side pointer_head=None, matching the pattern the two sibling stubs (test_hidden_state_server_mode, test_sampling_mask_validation) already adopted. 1F -> file GREEN. RUNNER-ZONE MERGE DAMAGE (attributed AT ae6ee52 in a scratch worktree: identical 7F there, NOT my regressions; tsv flashinfer_autotune/decode rows were B4-classified without a tree marker): - decode_cuda_graph_runner: _init_model_hooks DEFINITION lost at the merge while its __init__ CALL and the replay _replay_prepare_hook CONSUMER survived = every real DecodeCudaGraphRunner build dies AttributeError (boot-integrity class, same shape as B3-8's pool-side half). Re-planted VERBATIM from the entry-61 version (validate_model_support + the needs_replay_prepare-gated prepare_cuda_graph_replay hook; qwen4_exp still defines both). - test_flashinfer_autotune_gate 5F: upstream grew TWO post-suite arms (kv-sharding get_parallel read + the block-FP8-as-MXFP8 model walk); pinned both in _should_run per the file's OWN per_tensor_fp8 precedent. - test_graph_routing_context 2F: brand-new upstream file driving the fork-instrumented replay(); added the entry-61 hook off-pin to the __new__-built stub (entry-198 fixture-adaptation class). PER-MARKER DECISIONS: - prefill_cuda_graph_runner (300): RE-PLANT the ONE dropped line, the validate_model_support() call before capture (base_runner's method + model_context/capture/_capture/replay-context arms VERIFIED PRESENT -- entry-300 union landed at merge). - model_runner (300,187/sgl-project#43527): sgl-project#43527 half done IN B4-1 (allocator= kwarg); entry-300 half VERIFIED PRESENT (get_model().override(shared_experts=...) at split_prefill + forward ctx). - eager_runner (61,176): fold VERIFIED LANDED (model_context execute, shared_read_event import retarget, forward_context(get_forward_context()) decode wrap, resolve_prefill_shared_read_ends rename, cp_extend pair). - runner_utils/__init__ (176): SUPERSEDED BY DESIGN -- upstream made the package initializer import-free (its own docstring says so) and ALL consumers import defining submodules (grep-audited, incl. the cp_extend pair used by eager/base runners). - custom_all_reduce_v2 (75): SUPERSEDED with evidence -- upstream's own de26bb1 (sgl-project#46853) REMOVED the multi-node VMM allocator gate in-range (absent on origin/main); entry 75's LEGACY guard + shared probe live in custom_all_reduce.py:106 + cuda_vmm_utils:102; unit/distributed 156P/1S/ 35sub unmoved (the entry 308/309 guards stay intact). - logits_processor (162): RE-PLANT the one dropped arm, pcie_ipc=True on the vocab _logits_gatherer (the MultimemAllGatherer pcie_ipc API + whole transport stack survived the merge -- only this opt-in call kwarg was lost); the out=/fp32-buffer/cast_to_float32 arms VERIFIED PRESENT. pcie selection/ar tests green. - loader (178): RE-PLANT the sgl-project#42419 kv-scales-drop block (entries 178's recorded net-diff half) between the mxfp8 rename and the new-in-range monokernel wrapper; shared_experts resolve/override/attr arms VERIFIED PRESENT. unit/model_loader 161P/19S/91sub green. - serving_responses (110,201): RE-PLANT both _preprocess offloads (the PreprocessExecutor machinery landed complete via upstream's sgl-project#39716 absorption -- serving_base._preprocess + tokenizer_manager wiring + executor tests green); ADAPTED the _process_messages site with functools.partial to carry upstream's new-in-range output_constrained kw-only arg through the positional-only run() surface. - qwen3_coder_detector (20,21,89,108-109,157-160): fold VERIFIED LANDED (math/jsonschema/referencing imports, tuple-return _get_arguments_config, _parse_finite_float, anyOf/oneOf union arm, parameters threading at both sites) -- merged with our streaming-string region (561 discards parameters DELIBERATELY, our entry-159 path needs config only). - validation_hook (44,51) + moe_hook (247): VERIFIED -- union landed complete (no lost defs by AST diff; NGRAM+replayssm reject, b12x validator + FLASHINFER_B12X_W4A16 enum present; server_args 7F stays B6's re-pin). - component_manager (140): RE-PLANTED IN FULL from the 309 tip -- the merge DROPPED the whole parallel-stage half while parallel_executor.py CALLS supports_parallel_stage_group/parallel_stage_group (LIVE runtime crash class) + AST-proved the ported methods byte-equal, _uses_seen/ ResidentStrategy/ResidencyState(batch_is_warmup) all present, the 309->HEAD removed WarmupPhasePeak trio has ZERO callers (upstream's own in-file deletion). Diffusion import chain blocked at this seat by the missing imageio wheel (ENV class -- diffusion never ran here); battery = AST + py_compile + the executor's own unit contract file. - mm scheduler (133,137): fold VERIFIED LANDED (all upstream-side symbols present: _wait_idle/_FINALIZE_IDLE_WAIT_S/release_wait_s/progress_reporter). ACTUALS (damage -> after, CVD=9, prod venv, per-file): test_io_struct 1F -> ** 68P/2S/10sub ** unit/model_executor/runner ** 7F -> 103P/2S/73sub EXACT-GREEN ** (7F proven merge-era at ae6ee52 in a scratch worktree) unit/model_loader 161P/19S/91sub GREEN; unit/distributed 156P/1S/35sub UNMOVED; pcie selection+ar GREEN. DIR unit/managers 1025P/83F/9S/427sub -> ** 1056P/2F/9S/474sub ** (the 2F = the two DOCUMENTED order-dependent test_mm_process_config ENV-class F's, reproduce at base -- damage-map population preserved). DIR unit/spec 363P/20F/21S/238sub/2err -> ** 388P/0F/21S/238sub/0err ** (seven consecutive full-dir runs EXACT; the one unrepeated 1F sighting during iteration is recorded as a non-reproducing flake, candidate class = the same cross-file module state as managers' mm_process_config pair). test_pcie_ipc_vocab_gather 27F IDENTICAL at ae6ee52 = register_cuda_ci 4-gpu-b200 file run without GPU (ENV class, not this batch's dirs). serving_responses module import blocked by openai 2.6.1 vs required ==2.8.0 (upstream's own beam.py, pre-existing at merge -- ENV class). DoD(c) FULL-DIR totals recorded above; DoD(d) trailing_endpoint re-run ** 17P = 17P ** post-B4 (B3 gate unmoved). Lint: ruff format + F821/E9 clean across all touched files. No GPU; no push. Co-Authored-By: OpenCode <noreply@opencode.ai>
fixes #42369. drops fp8 kv scales at load time if runtime dtype is nvfp4. without this they get systematically mis-scaled (e.g. amax/448 vs amax/6). falling back to dynamic scales works correctly.
CI States
Latest PR Test (Base): ❌ Run #39578296118
Latest PR Test (Extra): ❌ Run #39578296451
Latest PR Test (AMD ROCm 10): ❌ Run #39578296354