Skip to content

docs(sglang): record the MTP penalty-accounting defect we filed but do not patch - #87

Draft
dorado269 wants to merge 1 commit into
mainfrom
docs/upstream-status-penalty-pr
Draft

docs(sglang): record the MTP penalty-accounting defect we filed but do not patch#87
dorado269 wants to merge 1 commit into
mainfrom
docs/upstream-status-penalty-pr

Conversation

@dorado269

Copy link
Copy Markdown
Collaborator

What

One row added to deploy/docker/patch.upstream.status.md for an sglang defect I filed upstream as sglang#33643 (ours, dorado269, DRAFT) and deliberately did not patch locally.

The defect: a decode step commits accept_len tokens under MTP, but cumulate_penalty_output_tokens feeds only output_ids[-1] to the sampling penalizers. Every other accepted token never enters penalty state and is never backfilled, so frequency_penalty ends up effectively divided by the accepted length, and min_new_tokens counts decode steps instead of tokens — masking EOS to -inf for roughly accept_len× longer than the request asked.

Why it needs a new section

The page opens with "Every patch under deploy/docker/patches/", and every section is a patch directory. This row has no patch file, so there is no patch header to hold the argument the way ## Maintenance assumes. Rather than bend an existing table, this adds ## Filed upstream, not patched here before ## Not patches, and says in the intro and in ## Maintenance that the section runs the other way.

The point of the row is that "we know, and here is why we are not carrying a diff" should be answerable from this page — otherwise the next person who finds #33643 re-argues it.

Why no local patch

The path is gated. eagle_prepare_for_decode calls cumulate_penalty_output_tokens() only when penalizer_orchestrator.is_required, which needs some in-flight request to carry a non-default frequency_penalty, presence_penalty, repetition_penalty or min_new_tokens. Nothing in this repo sets one — grepped 2026-08-05, the only penalty hits are unrelated prose about load and seek penalties.

The non-obvious way it could turn on is the model rather than our configs: --sampling-defaults defaults to model, so sglang copies repetition_penalty out of the served weights' generation_config.json into every chat request's defaults. Upstream zai-org/GLM-5.2 carries only temperature and top_p there, read from the raw file rather than inferred from a search, so the gate stays shut. We serve the MXFP4 derivative and it was not on the host to check — that caveat is stated in the row rather than glossed.

Verification

claim how
nothing in this repo sets a penalty grep across the tree, 2026-08-05
zai-org/GLM-5.2 has no repetition_penalty default raw generation_config.json fetched 2026-08-05
--sampling-defaults defaults to model read server_args.py in sglang main
#28180 and #29512 still OPEN, titles as quoted gh issue view, 2026-08-05
MXFP4 weights' generation_config.json not checked — weights not on the host, and the row says so

Docs only, no code or Dockerfile touched. Draft because #33643 is itself a draft; the PR state column should be refreshed if that changes before this merges.

…o not patch

sglang commits accept_len tokens per decode step under MTP but feeds only
output_ids[-1] to the sampling penalizers, so every other accepted token
never enters penalty state. Filed as sglang#33643 (ours, DRAFT).

Not carried as a local diff: cumulate_penalty_output_tokens runs only when
penalizer_orchestrator.is_required, and nothing here sets a penalty. The
gate that keeps it dead is the model, not us -- --sampling-defaults defaults
to `model`, so a repetition_penalty in the served weights' generation_config
would turn it on for every chat request. Upstream zai-org/GLM-5.2 carries
none; our MXFP4 derivative was not on the host to confirm.

Adds a "Filed upstream, not patched here" section for this class of row,
since the page until now assumed every entry had a patch file to hold its
argument.

Co-authored-by: Cursor <cursoragent@cursor.com>
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