misc: fix typo by zhyncs · Pull Request #698 · sgl-project/sglang

zhyncs · 2024-07-22T16:00:08Z

Thank you for your contribution, we really appreciate it. The following instructions will help improve your pull request and make it easier to receive feedback. If there are any items you don't understand, don't worry. Just submit the pull request and ask the maintainers for help.

Motivation

Please explain the motivation behind this PR and the goal you aim to achieve with it.

Modification

Briefly describe the changes made in this PR.

Checklist

Ensure pre-commit pre-commit run --all-files or other linting tools are used to fix potential lint issues.
Confirm that modifications are covered by complete unit tests. If not, please add more unit tests for correctness.
Modify documentation as needed, such as docstrings or example tutorials.

Update test_npu_kimi_k2_6_w4a8_16p_in64k_out1k_100ms.py

…le, falsifier PRIO. Deeper root beneath the sgl-project#698 wedge. Desk slices only; the wiring that changes serving behaviour is deliberately NOT in this commit. Root cause, one sentence: chunked prefill bounds the COMPUTE per step, not the KV COMMITMENT, and admission was reading the compute bound as if it were a memory decision. schedule_policy.py:1389-1407 charges the budget trunc_len -- one 512-token chunk -- while admitting a request whose real commitment is its entire remaining length. A 327,680-token request is therefore admitted on a 512-token affordability check, a 640x under-charge. The non-chunked branch directly above charges req.extend_range.length, the real figure; only the chunked path substitutes the chunk for the commitment. That explains the specimen without needing a second actor, which the specimen requires: ONE request (new-seq 1, new-token 512, cached 0) drove usage 0.95 -> 1.00 with no retract, abort or finish. Its own prefix locks as it grows, and a locked chain cannot be evicted to fund its own growth -- which is why the sgl-project#698 relief correctly reports "freed 0". sgl-project#698 made the failure legible; it could not fix it. Design decisions (DESIGN_701_chunked_admission.md): (a) Fund the full remaining length at admission; spill/retract of the request's own prefix comes later and is NOT a prerequisite. Funding is a correctness fix that cannot regress into a wrong answer, and it does not foreclose spill: a future spill capability simply raises the fundable total and the SAME rule then admits more. Stated honestly, this makes near-capacity requests slower -- some that are admitted today will defer. Trading throughput for not-deadlocking is the correct trade, and "freed 0" is the evidence that today's behaviour is a stall rather than a throughput win. (b) Head-of-line: refuse LOUDLY when the remaining length exceeds total pool capacity (can never fit at any future time), defer when it exceeds free + unlocked-evictable, admit otherwise. All three derived from pool arithmetic. There is deliberately no "90 percent of pool" style constant. (c) sgl-project#631 defect O expressed as one counting truth, effective_running_bs, so the ladder / delayer / idle-flip consumers converge on it instead of each re-deriving that a resident-but-batchless request means idle. planner/chunked_admission.py carries no rig threshold, no tuned fraction and no hardware or model name, per the binding generality clause. It has NO chunk-size parameter at all: the chunk is exactly what the old code substituted for the commitment, so the substitution is made unrepresentable, and a test asserts that passing chunk_tokens raises. Falsifier as specified by the ticket: a request whose remaining length exceeds free + unlocked-evictable must be refused or deferred, never admitted. Plus the specimen as a regression, and a generality test pinning that a request at 99 percent of a large pool admits while one at 101 percent of a small pool refuses, under the same function with no constant between them. Tests: 9, red first. Planner suite 2620 passed, 2 failed -- both the pre-existing test_rejected_evidence_pins failures verified earlier against clean integration/r2. ruff clean. NOT INCLUDED, and requires F4-r4 coordination before any deploy: wiring the rule into schedule_policy.py's chunked branch, which changes admission on the serving line.

…s standing in front of 54 MINUTES OF SILENT OUTAGE, health 200 throughout. From 16:23:10, 325 consecutive lines of: BOTH BLOCKED: nothing can run in the pp layout and the target cannot admit either (0 req resident, 10495392 tok pending) Last real batch 16:23:11: full token usage 1.00, #running-req 0, mamba 0.17. The whole KV pool was radix cache with ZERO resident requests -- every row unlocked and evictable -- while 10.5M tokens queued and three GPUs sat at 0%. ONE SWALLOWED EXCEPTION. _post_evict_rows asked tree_cache.evictable_size(). MambaRadixCache does not return a number from that method; it raises NotImplementedError and says "use full_evictable_size() and mamba_evictable_size() instead". The probe caught it and used 0, so on the class this rig runs it returned `available` ALONE -- the exact error its own docstring warns about, committed three lines below the warning. At usage 1.00 that reads ~0, so every admissibility question answered no: pp could not admit, tp had nothing resident to decode, and sgl-project#688's BOTH BLOCKED branch declined. That branch returns BEFORE alloc_token_slots -- so the allocator was never reached, eviction never ran, and the unlocked cache was never freed. The receipt called it "an evict trigger" while no evict could occur, which is the same counter-vs-actuator shape as sgl-project#681/sgl-project#694 in a third place: a message naming an action nothing performs. CONFIRMED BY ABSENCE in the specimen (WEDGE-2026-08-16T1623Z.txt): zero RADIX SHAPE, zero "Out of memory", zero EVICTION UNDER-DELIVERED. The allocation path was never entered. py-spy shows the ranks spinning the event loop building nothing. THE TRAP IS ALREADY DOCUMENTED IN THIS TREE, at common.py:411-425, for these same two classes -- and I read that comment the same day while diagnosing sgl-project#694 and did not apply it here. The resolution order is now COPIED from there rather than re-derived, because two spellings of one rule is how this comes back. WHY NOW. The bug shipped with sgl-project#688's admissibility simulation and needed usage == 1.00 to bite. sgl-project#696's floor repair shrank the pool by 39,504 tokens, so full occupancy arrived sooner and the wedge began 12 minutes after that boot. sgl-project#696 EXPOSED this; it did not cause it. THE SHAPE TO LEARN: a swallowed exception that yields a PLAUSIBLE value. Zero is a legal row count, so nothing downstream could tell "the cache holds nothing" from "the cache was never asked". Every accessor is now tried in turn and only a genuine absence of all of them yields zero. Health being 200 for the entire outage is the second lesson: the endpoint answers while the scheduler builds no batch. It is not a liveness signal. TESTS: test_post_evict_rows_698.py 5 passed (red-first: pre-fix it fails "0 not greater than or equal to 150000", reproducing the wedge arithmetic). managers + mem_cache 64 failed / 1922 passed -- the same 64 baseline.

…es, and pin it THE INVARIANT WAS PROSE. phase_policy's branch says "Declining here is what routes the caller to the evict rung instead of to a cutover." The caller did not: scheduler.py handled every decline identically -- one throttled log line, return. So on 2026-08-16 the instance printed "this is an evict trigger and NOT a flip" 350 times across 54 minutes while no eviction was ever attempted, health answered 200 the whole time, three GPUs sat at 0%, and 10.5M tokens queued behind a pool nothing would free. I wrote that comment in sgl-project#688. It is the fourth counter-vs-actuator member found today, and the first where the actuator existed only as a sentence. WHAT THIS COMMIT DOES: the decline now calls evict_from_tree_cache, bounded to one attempt per 5 s (the decline is evaluated every round, so an unbounded call walks the whole tree in a tight loop on an already-wedged box), and REPORTS WHAT EVICTION RETURNED. THE REPORT IS THE POINT AS MUCH AS THE CALL. "The remedy ran and freed 0" and "the remedy never ran" are the two states this outage could not distinguish, and telling them apart is what turns a silent wedge into a diagnosis. AND ON THIS SPECIMEN IT WOULD HAVE DELIVERED ZERO -- said plainly, because the routing is necessary and NOT sufficient. At 16:22:58-16:23:11 a single request was mid CHUNK-PREFILL (#new-seq 1, #new-token 512, #cached-token 0) while usage climbed 0.95 -> 1.00, with no retract, abort or finish anywhere in the window. A chunked request is resident but sits in NO BATCH (sgl-project#631 defect O), which is why the scheduler read #running-req: 0 while its own protected prefix held the pool. Eviction cannot free a locked chain. The deeper root is that a chunked request's prefix can fill the pool its next chunk must allocate from -- a self-deadlock -- and that is a separate fix on the chunked/admission path, not this one. Also exports BOTH_BLOCKED as a shared constant instead of matching a respelled literal across two modules. TESTS: test_both_blocked_routing_698.py 6 passed -- the sgl-project#505 discipline: the pin fails if the caller stops routing, if the relief stops calling eviction, if it stops reporting a zero delivery, or if the rate limit is removed. managers + mem_cache 64 failed / 1928 passed, the same 64 baseline.

… name not the call form Two follow-ups to 66382c2, whose stated 64/1928 was wrong when written -- it was 68, and I should have re-read the number before claiming it. 1. The policy gate is driven in tests by scheduler STAND-INS carrying only the fields the policy reads, so calling the new hook directly raised AttributeError in the arming path and red four test_phase_policy cases. Same shape, same file, THIRD time today (_idle_locked_inputs, the cold-branch names, now this). Invoked through getattr with a noop default: a stand-in without the hook declines exactly as before. 2. That made my own pin fail: routing is no longer an ast.Attribute call, so an AST walk keyed on attribute calls missed it. The pin now matches the identifier anywhere in the function source, which survives either spelling and still fails if the routing is deleted -- the only thing it is for. managers + mem_cache 64 failed / 1928 passed = the baseline, verified before this message was written.

…gl-project#701 ## The gate, respecified so it can actually pass DESIGN_704 originally demanded byte-identity "decoupled vs coupled". That is A-vs-B, not A-vs-A: an LSE merge sums partials in a different floating-point order than monolithic attention, so bit-exact agreement is not a property correct code has. As written the gate would fail forever on a correct implementation, and the predictable outcome is that someone waives it -- worse than having no gate. planner/lse_merge_gate.py harnesses both halves: GATE 1, DETERMINISM, byte-identical A-vs-A, never waivable. The same inputs merged twice must be bit-identical. This catches the most likely silent defect in a distributed merge -- folding partials in ARRIVAL order rather than rank order, which yields a different rounding every run and is invisible in any single run. No tolerance is applied, because a tolerance would hide exactly what the gate is for. GATE 2, AGREEMENT with the coupled reference within a tolerance fixed BEFORE the run. The tolerances are arguments with no defaults on purpose: a tolerance chosen after seeing the numbers is not a gate. merge_partials() is the CONTRACT the GPU path must satisfy, written to match layers/dcp/comm.py:228-262 (cp_lse_ag_out_ar_mha_uneven): all-gather every rank's LSE, reduce with one logsumexp over the stacked axis, so merge order is RANK order fixed by the collective and never arrival order. Inputs are sampled on CPU by construction, not by convention: torch.randn on-GPU is not architecture-identical across the 3080s and the 5090, and a harness that seeded on device would make gate 1 fail for a reason that has nothing to do with the merge. Deliberate asymmetry: given rank_order the merge reorders before reducing; WITHOUT it, list order IS taken as rank order. A caller that shuffles and stays silent has a bug, and the harness must not launder it into a plausible answer. ## The D6 admission hold shares a failure mode with a live bug Recorded as a hard precondition. The sgl-project#701/sgl-project#698 chunked-prefill admission deadlock is currently the dominant live defect -- #running-req: 0 on 90.6% of prefill rounds, zero completions -- and it is the SAME wedge D6 describes, already happening for another reason. An admission hold dropped into an admitter that is already starving would deepen the deadlock rather than bound a drain, and would then be indistinguishable from it in a log. So the hold ships GATED: it may fire only when admission is demonstrably live, and is disabled outright until sgl-project#701 lands. My mechanism and an existing bug share a failure mode, and the ordering between them is not optional. ## Instrumentation warnings carried into the design cache_hit_rate reports 0.0 despite real hits (separate filed bug) -- count hits from log lines and token counts instead; a gate written against that counter would pass or fail for reasons unrelated to what it measures. And acceptance of the "real cache hit across flip AND reboot" kind is unfalsifiable until sgl-project#701 is fixed, because the cache is STARVED rather than broken: such a test would fail for the wrong reason and must not consume a boot window. ## Test results 138 passed, hermetic (CUDA_VISIBLE_DEVICES=""), ruff clean, codespell clean. test_lse_merge_gate_704b.py (9, new): bit-identity across repeated runs; inputs CPU-sampled and seed-reproducible; merge order is rank order not arrival order; a silent shuffle is caught rather than tolerated; assert_deterministic carries a CAN-FAIL proof against a deliberately flaky merge; one partial returns unchanged; the sharded merge reproduces a monolithic softmax to 1e-10 on float64; the agreement report gates on a pre-fixed tolerance; shape mismatches refused.

…e-key its own pin Merging Slot-3's sgl-project#713 admission-intake work surfaced two defects of mine, both of which I had already learned once in this same series. 1. THE STAND-IN TRAP, third time. sgl-project#708 wired the policy input as a direct call, self._uniform_kv_available(). Slot-3's scheduler STAND-INS carry only the fields the policy reads, so the merge produced nine failures with "'S' object has no attribute '_uniform_kv_available'". _idle_locked_inputs and the both-blocked relief were both given getattr defaults earlier for exactly this reason; I then wrote new code without applying my own lesson. Now getattr(self, "_uniform_kv_available", lambda: None)() -- a stand-in without the probe has MEASURED NOTHING, and "not measured" is a state the sgl-project#708 branch already reports honestly, so the degradation is free. 2. MY OWN PIN PUNISHED THE FIX. test_scheduler_passes_kv_available_tokens asserted the literal string "kv_available_tokens=self._uniform_kv_available()" and so went red on the legal refactor above -- the same way the sgl-project#698 pin broke when that call was made defensive. Re-keyed on the IDENTIFIER plus the parameter name rather than the call form. A pin must survive a legal refactor of the thing it pins, or it punishes the repair instead of the regression. Also recorded honestly: I pushed the merge before reading its test result, because I chained the push after pytest with ';' rather than '&&'. The branch carried nine failures for a few minutes. Sequence the push behind the tests. Tests: 190 passed + 21 subtests across admission-intake, phase-policy, 708, 713, 699 and flip-runtime. ruff clean in the changed region.

misc: fix typo

5295641

zhyncs merged commit 9fdea29 into sgl-project:main Jul 22, 2024

zhyncs deleted the hot branch July 22, 2024 16:00

timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025

misc: fix typo (sgl-project#698)

379c617

hhhh1252023 pushed a commit to hhhh1252023/sglang_public that referenced this pull request Jun 5, 2026

Merge pull request sgl-project#698 from hhhh1252023/patch-5

3ea9dbc

Update test_npu_kimi_k2_6_w4a8_16p_in64k_out1k_100ms.py

Navigation Menu

Uh oh!

misc: fix typo - #698

misc: fix typo#698
zhyncs merged 1 commit into
sgl-project:mainfrom
zhyncs:hot

zhyncs commented Jul 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

zhyncs commented Jul 22, 2024

Motivation

Modification

Checklist

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant