feat(fp8-dense): nvfp4 프리필 경로를 fp8 메서드에 부착 (레버 7, 노브 off) - #296
Conversation
…er 7, knob off) The nvfp4 scheme replaces the method and so turns the MK W4 lane off for every layer it takes (#263). This attaches an nvfp4 pair to the fp8 method instead: rows above the MK lane's M (32) route to mm_fp4 in Fp8DenseMethod.apply, decode keeps the W4 lane, the fp8 pair stays the fallback (a failing pair drops that layer's prefill to it for good, loudly). Alpha convention settled once, a sample of layers re-runs the value check, as the scheme does. Target only. Knob VLLM_GLM53_FP8_DENSE_PREFILL_NVFP4 (default 0): A4 activations are a served-numerics change, so the bracket (ladder + quality + Korean + acceptance profile) decides. The boot fingerprint counts the pairs. tests: load_defs extracts classes; routing by rows, bias/opaque paths untouched, failure drops to fp8, knob exactness and target-only, build wiring, profile default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_da023b46-1001-4433-9332-c5040712dbc0) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
The knob’s documented “exact ‘1’ arms” contract is inconsistent with the implementation, and the updated test loader still reads source without an explicit UTF-8 encoding which can break on non-UTF-8 default locales.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds “lever 7” for GLM53 fp8-dense: when enabled, eligible fp8-dense linears attach an nvfp4 prefill pair and route prefill shapes (M > 32) through mm_fp4 while keeping the existing MK W4 lane / fp8 behavior for decode and opaque (drafter) paths. This is default-off, documented as target-only, and includes tests and boot fingerprint logging for “N nvfp4 prefill pairs”.
Changes:
- Extend
Fp8DenseMethodwith an optional_nvfp4pair and route prefill rows to_nvfp4_dense_gemm_op, with per-layer permanent fallback on runtime failure. - Add
_nvfp4_pair_for()+ build-time wiring to attach pairs when the knob is enabled and K is nvfp4-block-aligned; include fingerprint logging. - Update docs/profile defaults and add a targeted unit test; enhance
load_defs()to allow extractingClassDefs for tests.
File summaries
| File | Description |
|---|---|
| tests/test_logic.py | Teach load_defs() to load classes; add a unit test covering row routing, opaque/bias behavior, fallback demotion, and knob wiring. |
| profiles/glm53.env | Document lever 7 and add default VLLM_GLM53_FP8_DENSE_PREFILL_NVFP4=0. |
| overlay/modules/glm53_fp8_dense/README.md | Document the new prefill-on-nvfp4-pair knob, behavior, cost, and gating. |
| overlay/modules/glm53_fp8_dense/glm53_fp8_dense.py | Implement the knob, pair building, routing behavior, and fingerprint logging in the overlay module. |
| build/glm53/glm53_fp8_dense.py | Mirror the same implementation for the build-time copy of the module. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _prefill_nvfp4_enabled(env: str) -> bool: | ||
| if env != "VLLM_GLM53_FP8_DENSE": | ||
| return False | ||
| raw = (os.environ.get(_PREFILL_NVFP4_ENV) or "0").strip().lower() | ||
| return raw in ("1", "true", "on", "yes") |
| def _prefill_nvfp4_enabled(env: str) -> bool: | ||
| if env != "VLLM_GLM53_FP8_DENSE": | ||
| return False | ||
| raw = (os.environ.get(_PREFILL_NVFP4_ENV) or "0").strip().lower() | ||
| return raw in ("1", "true", "on", "yes") |
| @@ -77,7 +77,7 @@ def load_defs(relpath: str, names: set[str], ns: dict) -> dict: | |||
| tree = ast.parse(open(path).read()) | |||
레버 7 (28차 목록): nvfp4 를 프리필 전용 경로로 fp8 메서드에 붙입니다. nvfp4 스킴은 메서드를 통째로 바꿔 MK W4 레인을 끄지만(#263), 이 경로는 fp8 메서드와 W4 팩을 그대로 두고 M>32 행만
mm_fp4로 보냅니다(디코드는 레인, 실패 시 그 층의 프리필만 fp8 쌍으로 영구 강등·로그). 알파 관행 1회 확정 + 표본 값 검사는 스킴과 동일. 타깃 전용. 노브VLLM_GLM53_FP8_DENSE_PREFILL_NVFP4기본 0 — A4 활성화라 브래킷(사다리·품질·한국어·수용률 프로파일)이 결정합니다. 핑거프린트에 "N nvfp4 prefill pairs".테스트 44,576 checks OK(로더가 클래스도 추출; 행 수 라우팅·bias/opaque 경로 불변·실패 강등·노브 정확성).
🤖 Generated with Claude Code
Note
Medium Risk
Changes served numerics for prefill (A4 activations) when the knob is enabled, with per-layer silent fallback on kernel failure; default-off limits exposure but the path touches hot dense GEMMs on long prompts.
Overview
Adds lever 7: an optional nvfp4 weight+activation (
mm_fp4) path on top of the existing w8a8Fp8DenseMethod, without replacing the method or turning off the MK W4 decode lane.Runtime: When
VLLM_GLM53_FP8_DENSE_PREFILL_NVFP4=1(targetVLLM_GLM53_FP8_DENSEonly) and a layer has a built_nvfp4pair,Fp8DenseMethod.applysends M > 32 token rows to_nvfp4_dense_gemm_op; decode (M ≤ 32) still uses MK W4 / fp8 as before. Bias, and the drafter opaque op path, are unchanged. A runtime failure clears_nvfp4for that layer and falls back to fp8 for the rest of the boot.Load time:
maybe_build_fp8_densecalls new_nvfp4_pair_for(same alpha resolution and sampled value checks as the full nvfp4 scheme) for K divisible by 16; boot fingerprint logs N nvfp4 prefill pairs. Profile default remains 0; README documents cost (~+1 GB/rank) and bracket gates.Tests:
load_defscan extractClassDef; new test covers row routing, opaque/bias paths, failure demotion, and knob wiring.Reviewed by Cursor Bugbot for commit b44b00a. Bugbot is set up for automated code reviews on this repo. Configure here.