clean up step function by Ying1123 · Pull Request #635 · sgl-project/sglang

Ying1123 · 2024-07-17T03:15:18Z

No description provided.

… J's root cause Supersedes the defect-G and acceptance-readiness sections of the original sgl-project#656 handoff. Records: G fixed and proven on metal; the first policy-driven flip in the feature's history committing at 01:37:12Z; the self-contradictory quiescence predicate that was the real unblocker; and defect J in three parts -- J.1 slot scope (proven, fixed), J.2 row extent (measured, deliberately not cut, with the live-spec-reserve measurement owed before it is), J.3 the cutover not carrying the resident decode set (root cause, and the reason a flip under load is currently impossible). Names the next build precisely -- resident-request carry across the stack/topology swap in build_production_flip_cutover and the scheduler topology snapshot, covering request objects, scheduler bookkeeping and mamba/GDN state AND locks, not just KV cells -- and flags the standing architecture context that design must compose with (sgl-project#635/sgl-project#636 PP dcp_size=1 to TP dcp_size=3 handover and its four silent preconditions; sgl-project#212 store routes truncating GDN state). Also records the two hypotheses of mine that died on the way, so they are not re-derived from a symptom they both fit perfectly, and the SGLANG_ENABLE_STRICT_MEM_CHECK_DURING_IDLE=0 demotion that unmasked J.3. Documentation only; no behaviour change. Suite unchanged at 379 passed.

…t_length, not a KV reserve Measured on the proven boot: one 131072-token session with no other traffic peaks at 'full token usage: 0.50', so the denominator is 262144 (--context-length), not the 459392 global pool. 131072 is already 0.285 of the global pool, past the 0.25 that a held-back reserve for requests 2-4 would imply. So at bs1 the session is not capped by KV reserved for the other three slots; it is capped by the per-session context ceiling, leaving ~197k tokens of pool it cannot address. Growing the single session's usable KV is therefore the same action as the YaRN leg, via the sgl-project#543 split, and the existing pool supports a >262144 session without the sgl-project#297/sgl-project#635 rebuild. What is still spill-shaped is the mamba/GDN state pool (20 slots, ~5 per request, 15 idle at bs1), whose VRAM would have to be converted into KV pool at runtime to become usable context. Not evidenced yet.

The inversion this slice exists for. Today model_runner_kv_cache_mixin.py:2496-2500 filters the model's attention layer ids to [start_layer, end_layer) BEFORE the pool is built, so a rank has no row-space for a layer whose weights it does not hold: stage-local: own_attn_layers x ALL tokens decoupled: ALL attn_layers x own token share 13 hermetic tests. PLANNING AND VALIDATION ONLY -- no pool is built and nothing is armed; caller wiring stays out of scope until this lands, as instructed. ## Neighbours: nothing to reuse, and I checked before deriving git log --all --grep for [sgl-project#646] and [sgl-project#635] returns NOTHING reachable from this branch, and no design docs exist for either in docs/ or evidence-665-f1/. So the flat-half-split fix and the handover analysis are not available to me here. I did not re-derive them; R6 stands on the ownership filter I located directly. If those commits live on another strand's unfetched branch, this plan should be re-checked against them before wiring. ## The invariant worth holding World total is CONSERVED between modes: stage-local sums to sum(own_attn_i) x T = 16 x T, decoupled to 16 x sum(share_i) x T = 16 x T. Decoupling REDISTRIBUTES capacity; it does not create it. A share vector that does not sum to one silently shrinks the world pool, so validate_world_conservation catches it by name. ## The share is NOT the layer fraction, and that is the point rank0 owns 7 of 16 attention layers (43.75%) but takes a 13.5% token share -- the free-proportional vector from #704b. Deriving the share from layer count would rebuild exactly the ownership coupling this removes, so plan_for_rank REFUSES to default it: armed without a share raises, and a share supplied while unarmed raises too (a share that silently does nothing is how a decoupled pool ends up sized like a stage-local one). ## Loud failure, proven by breakage not asserted A rank holding only its own layers cannot answer a read for a layer another stage owns, and that surfaces as WRONG OUTPUT rather than a missing row. validate_plan rejects it by name. Proof it bites: neutering the two checks turns exactly two tests red (wrong-sized armed pool, under-summing share vector); reverting restores 13 passed. Unarmed byte-identity is pinned positively -- attention counts 7/5/4, tokens = T, bytes = attn_i x T x cell -- not merely "similar". ## Seam to sgl-project#706, stated precisely because two nearby facts point opposite ways layer_extents() expresses the plan in the canonical store's terms and uses the GLOBAL attention slot index, never a rank-local one -- the same silent failure shape the canonical-page work already found. Under decoupling a rank covers every slot, so RESIDENCE IS A WHOLE PAGE: a page never straddles ranks, which is what the canonical store wants. That does NOT remove the completeness marker. PRODUCTION stays layer-sharded -- stage 0 computes layers 0-27, stage 1 the next span -- so a page's 16 slots still arrive from three writers. Residence is whole; authorship is not. Both of my earlier statements hold; they were about different things. Coordinated via this spec only; sgl-project#706's branch is untouched.

…are the owner rule cannot express REVIEW correction first: 4fdefe8's message reported the sgl-project#635/sgl-project#646 neighbour work as unreachable from this branch. That flag was WRONG and is withdrawn. The miss was a search error -- I grepped the decorated forms "[sgl-project#646]"/"[sgl-project#635]" while the subjects use "sgl-project#646:" and "(sgl-project#635)". Canon: grep ticket numbers bare, then filter the noise; never narrow by adding decoration. Verdict on 7fd2b56 "R6 verdict (sgl-project#635)": COMPLEMENTARY, not a duplicate, and it is a DIFFERENT R6 -- a risk-register item in docs/dev/DESIGN_625.md about the PD handover between two engines, already superseded there by R7. This R6 is a #704b slice about pool allocation inside one engine. It IS an ancestor of this branch, so the delineation is real but the excuse was not. They meet on the token-sharded pool: sgl-project#635 says how bytes ARRIVE into one (base/conn.py:186-204, decode.py:1103-1125); this slice says how one is SIZED. That meeting point exposed a real defect rather than a tidy fit. The defect: the owner rule assigns slot L to a rank iff (L % cp_S) is in [cp_lo, cp_hi) (layers/dcp/owner.py:406-436), so a realized share is exactly (cp_hi - cp_lo) / cp_S -- a rational with denominator cp_S, never a free real. plan_decoupled accepted any float in (0, 1] and never checked it. A pool sized off that grid is sized for a token count the rank is never handed, and the mismatch is silent: loc = block * cp_ratio + (off - cp_lo) indexes an allocation never made that long. Worse, this contradicts my OWN earlier work -- planner/decoupled_kv.py:270-273 already states the ratio_r / S constraint in quantize_shares. A self-consistency failure, not a missing external fact. Fix: period is now REQUIRED at arming (plan_decoupled, plan_for_rank), and validate_share_realizable REFUSES an off-grid share rather than rounding it; rounding would hide that the caller skipped quantize_shares. realized_share() reads the share off the owner rule's own expression so the planner is not a third copy of it. Verdict on b851df7 (sgl-project#646): COMPLEMENTARY; the defect is NOT recreated and the rule is adopted anyway. sgl-project#646 is NOT an ancestor of this branch (it lives on feat/dual-group-631; num_target_kv_buffers greps empty here). Its defect was a flat list that stopped being one section once the draft pool was appended, after which a halved boundary mispaired source V with destination K silently. This module consumes a different list (model_runner_kv_cache_mixin.py:2496-2500, which the draft pool does not enter), so it does not inherit that defect. But the armed path shards by token share whatever it is handed, and an appended section would be split across ranks when sgl-project#646 established such a section is owned WHOLE by the last PP stage. _validate_target_section refuses a repeating or non-increasing list by name. Armed path only; a test pins that the unarmed path did not acquire the new refusals. Test results (hermetic, CUDA_VISIBLE_DEVICES=""): test/registered/unit/mem_cache/test_decoupled_kv_pool_plan_704b.py 13 passed -> 23 passed can-fail by breakage: neutering validate_share_realizable and _validate_target_section turns EXACTLY 5 tests red (share_OFF_the_owner_rule_grid, names_the_grid_it_wanted, REFUSED_not_rounded, SECOND_SECTION_appended, concatenated_layer_list); restoring returns 23 passed, no collateral same share, different period: 0.135 accepted at period=1000 (135/1000), refused at period=3 -- the period decides, not the share's plausibility test/registered/unit/{mem_cache,planner} under /spinning/htsglang-gpu/.venv/bin/python3 2836 passed, 123 skipped, 157 subtests passed, 2 failed Two facts found while validating, neither caused by this change: 1. System python3 can no longer COLLECT 67 of these suites (ModuleNotFoundError: datasets, and chess). The venv at /spinning/htsglang-gpu/.venv/bin/python3 still has them. A "green" run on bare python3 after this drift is a collection error, not a pass. 2. The 2 failures are a drifted evidence pin, pre-existing at HEAD (verified by removing this diff and re-running): planner/rejected.py:361 cites server_args.py:16240-16245 (assert), but that region is now _handle_tokenizer_batching. The pinned assert moved to server_args.py:13693-13694 and changed shape (view.disable_overlap_ schedule, not self.). Not mine, not fixed here. Still planning and validation only. Nothing armed, no pools built.

…-pointed This was assigned as a pin-only line-number fix, on the strength of a diagnosis I gave in the previous report. That diagnosis was WRONG and is withdrawn: I said the pinned assert had moved to server_args.py:13693-13694 and changed shape to view.disable_overlap_schedule. That site is _validate_mamba_no_buffer, an unrelated mamba assert. I read a bare disable_overlap_schedule grep hit as the pinned guard without reading its context -- the same infer-from-a-hit error sgl-project#635 named ("a constant is evidence about the code that reads it and nothing else"). Reading the real guard shows the row was not merely mis-pointed. The single conjunction the row describes has been SPLIT INTO TWO statements that went in OPPOSITE directions: server_args.py:17583 if self.pp_size > 1: server_args.py:17584-17586 assert self.disable_overlap_schedule server_args.py:17598 assert self.speculative_algorithm is None or self.enable_phase_flip 1. The speculation half is still a hard assert, but it GREW AN EXEMPTION: "or self.enable_phase_flip". Per the comment at :17587-17597, a phase-flip instance runs PP for prefill with no draft worker built at all and arms speculation on the TP decode stack at cutover, so the combination is enforced by construction instead of refused. 2. The overlap half is now effectively an auto-disable. arg_groups/ overrides.py:2163 _pipeline_parallel_overlap_disable sets disable_overlap_schedule=True and logs a warning in a post-process pass that runs BEFORE the assert, so that assert can no longer fire. So the row's claim -- "a hard assert, not an auto-disable ... every PP number is therefore a no-spec number" -- was wrong in both halves, and wrong in the direction that matters to this strand: it forbade on paper the exact configuration sgl-project#704 is built on (PP=3 prefill flipping to TP=3+EAGLE decode). Updating only the line number would have re-pointed a false verdict at the code that falsifies it. Row corrected: BLOCKED for PLAIN pipeline parallelism, EXEMPT under --enable-phase-flip, with the auto-disable of the sibling half recorded so the register does not imply both halves still refuse. Level unchanged at BLOCKED; whether a phase-flip-exempt row should keep that level is a register-owner call, not mine. Tests: the two reds were ready-made falsifiers, but they pinned the OLD single conjunction, so they could not express the split. Rewritten to pin both halves plus the exemption, each with its own falsifier proven by breakage: test_evidence_cites_land_on_BOTH_halves_of_the_guard red when the evidence carries the stale :16240-16245 cite test_the_register_records_the_PHASE_FLIP_EXEMPTION red when the row text is reverted to the pre-#704b wording test_the_overlap_half_is_now_an_AUTO_DISABLE red when the row stops naming disable_overlap_schedule. Note the weaker form of this check passed against the stale text: "auto-disable" appears there too, inside "not a quiet auto-disable" -- the opposite claim. Naming the field is what makes it a real check. test/registered/unit/planner/test_rejected_evidence_pins.py 2 failed, 2 passed -> 6 passed test/registered/unit/planner full suite, no remaining failures Hermetic, CUDA_VISIBLE_DEVICES="", interpreter /spinning/htsglang-gpu/.venv/bin/python3 (system python3 cannot collect these suites; see 2c0542e). Register text and its pins only; no engine behaviour touched. Register rows carry a length budget (test_wizard_v2.py:482-490: why <= 400 chars, gain/cost <= 120) -- "a paragraph in the gain field is how this becomes the wall it replaced". My first pass at the corrected row blew the why budget at ~600 chars and turned that test red; the detail moved into evidence, which carries no budget. why is now 295. Full planner suite: 2805 passed, 0 failed (2 failed at the parent commit).

clean up step function

6a8f74d

Ying1123 merged commit a470e60 into main Jul 17, 2024

Ying1123 deleted the cleanup branch July 17, 2024 03:15

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

clean up step function (sgl-project#635)

0d84cc7

Navigation Menu

Uh oh!

clean up step function - #635

clean up step function#635
Ying1123 merged 1 commit into
mainfrom
cleanup

Ying1123 commented Jul 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Ying1123 commented Jul 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant