Skip to content

[Fix] Drop the shadowed copies of the lm_head quant-method helpers - #39707

Open
JasonKeyiL wants to merge 1 commit into
sgl-project:mainfrom
JasonKeyiL:fix-logits-dup
Open

JasonKeyiL wants to merge 1 commit into
sgl-project:mainfrom
JasonKeyiL:fix-logits-dup

Conversation

@JasonKeyiL

@JasonKeyiL JasonKeyiL commented Sep 16, 2026 •

Copy link
Copy Markdown

Motivation

logits_processor.py defines _has_lm_head_runtime_attrs and should_apply_lm_head_quant_method twice at module level, at lines 123/127 and 1230/1234. Python binds the later definition, so the pair near the top of the file has never run.

The two copies are no longer the same. The duplication came in with the rebase in #35758 (2026-08-28), which left the file with two copies of each. Three days later #35120 added the NVFP4 W4A16 branch to the copy Python actually binds, and only to that one:

 if method_name == "ModelOptFp4LinearMethod":
+    if quant_method.quant_mode == "w4a16":
+        return lm_head.weight.dtype == torch.uint8 and _has_lm_head_runtime_attrs(...)

So the shadowed copy answers False for a w4a16 lm_head that the live one accepts. This is the predicate that decides whether a draft model may reuse a quantized target lm_head, and reading the top of the file currently gives the wrong answer about which layouts are supported.

ast.unparse of the two _has_lm_head_runtime_attrs bodies is identical, so that one is pure duplication.

Modifications

Remove the shadowed definitions at 123 and 127. Nothing else changes.

Accuracy Tests

No behaviour change, and there is nothing new to test: the definitions that survive are the ones Python was already binding.

Verified rather than assumed:

_has_lm_head_runtime_attrs:         ast.unparse(kept) == ast.unparse(main's last def)  -> True
should_apply_lm_head_quant_method:  ast.unparse(kept) == ast.unparse(main's last def)  -> True
module top-level symbols:           41 -> 39   (exactly the two removed)

test/registered/unit/spec/test_eagle_draft_extend_logits.py passes locally (5 tests), and should_apply_lm_head_quant_method still imports and evaluates as before. black --check and isort --check-only are clean.

Verified on macOS/arm64, Python 3.13, CPU only.

Checklist


CI States

Latest PR Test (Base): ❌ Run #35055132838
Latest PR Test (Extra): ❌ Run #35055132737
Latest PR Test (AMD ROCm 10): ❌ Run #35055132762

`logits_processor.py` defines `_has_lm_head_runtime_attrs` and
`should_apply_lm_head_quant_method` twice at module level. Python binds the
later definition, so the pair near the top of the file has never run.

The duplication came in with the rebase in sgl-project#35758 (2026-08-28), which left the
file with two copies of each. Three days later sgl-project#35120 added the NVFP4 W4A16
branch to the copy that Python actually binds, and the two diverged: the
shadowed `should_apply_lm_head_quant_method` has no `quant_mode == "w4a16"`
branch, so it answers False for a w4a16 lm_head that the live copy accepts.
Reading the top of the file now gives the wrong answer about which layouts are
supported, which matters because this is the predicate that decides whether a
draft model may reuse a quantized target lm_head.

`ast.unparse` of the two `_has_lm_head_runtime_attrs` bodies is identical, so
that one is pure duplication.

Remove the shadowed pair. The surviving definitions are identical under
`ast.unparse` to the ones Python already binds on main, so behavior does not
change.
@JasonKeyiL

Copy link
Copy Markdown
Author

Could someone add the run-ci label? /tag-run-ci-label is a no-op for me, so pr-gate blocks this.

It is a pure deletion of two module-level definitions Python never binds; ast.unparse of what survives matches what main already executes, so there is no behaviour to test.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant