Skip to content

[Quant] Reject non-finite and non-per-tensor FP8 KV scales at load - #41742

Open
rodamani wants to merge 5 commits into
sgl-project:mainfrom
modal-projects:rohan/up/fp8-kv-scale-validation
Open

rodamani wants to merge 5 commits into
sgl-project:mainfrom
modal-projects:rohan/up/fp8-kv-scale-validation

Conversation

@rodamani

@rodamani rodamani commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

FP8 KV-cache scale loading in layers/quantization/kv_cache.py rejects zero scales but accepts non-finite ones, so a checkpoint with a NaN / Inf k_scale or v_scale loads and silently corrupts attention. Multi-element (non-per-tensor) scales only fail later with an ambiguous RuntimeError from a tensor comparison.

Modifications

  • Check numel() == 1 first and raise a clear ValueError for non-per-tensor scales.
  • Reject non-finite scales with ValueError (evaluated on CPU copies).
  • test/registered/unit/layers/quantization/test_kv_cache_scale_validation.py.

#40243 proposed the finite check and was closed without merge.

Accuracy Tests

CPU: 7 passed. With the first commit's source change reverted, 3 of its 6 tests fail.

Speed Tests and Profiling

Load-time validation only.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ✅ Run #36767676542
Latest PR Test (Extra): ❌ Run #36767675843
Latest PR Test (AMD ROCm 10): ❌ Run #36767675960

rchalamala and others added 2 commits September 29, 2026 19:23
…scale check) (sgl-project#97)

* quant: reject non-finite fp8 K/V scales at load (sibling of the zero-scale check)

Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>

* kv_cache: raise ValueError for non-finite scales instead of assert

Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Relationship to upstream: sgl-project#40243 (draft, closed without merge
2026-09-25) proposed the same check; main still accepts non-finite scales.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…roject#99)

Check numel before the finite check so multi-element scales raise a
clear ValueError instead of an ambiguous RuntimeError, and evaluate
isfinite on CPU copies.

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Relationship to upstream: sgl-project#40243 (draft, closed without merge
2026-09-25) proposed the same check. Main only rejects multi-element scales
after .tolist(), after the comparisons that raise an ambiguous RuntimeError.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@rodamani

Copy link
Copy Markdown
Contributor Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants