Skip to content

Remove the dependency of rpyc - #646

Merged
Ying1123 merged 3 commits into
mainfrom
remove-rpyc
Jul 18, 2024
Merged

Ying1123 merged 3 commits into
mainfrom
remove-rpyc

Conversation

@wisclmy0611

@wisclmy0611 wisclmy0611 commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

refactor TP and DP workers to fully remove the dependency of rpyc

@Ying1123 Ying1123 changed the title Remove rpyc Remove the dependency of rpyc Jul 18, 2024
@Ying1123
Ying1123 merged commit d774aca into main Jul 18, 2024
@Ying1123
Ying1123 deleted the remove-rpyc branch July 18, 2024 09:13
@merrymercy
merrymercy restored the remove-rpyc branch July 18, 2024 09:14
@Ying1123
Ying1123 deleted the remove-rpyc branch July 18, 2024 09:14
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
cen121212 pushed a commit to cen121212/sglang that referenced this pull request Nov 10, 2025
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 17, 2026
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.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 17, 2026
…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.
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.

2 participants