support models from www.modelscope.cn - #994
Merged
merrymercy merged 7 commits intoAug 9, 2024
Merged
Conversation
zhyncs
reviewed
Aug 9, 2024
zhyncs
reviewed
Aug 9, 2024
zhyncs
approved these changes
Aug 9, 2024
zhyncs
left a comment
zhyncs
left a comment
Contributor
There was a problem hiding this comment.
@liuyhwangyh LGTM I will verify it locally, and if there are no issues, I'll merge it. Thanks for your contribution!
Contributor
|
@liuyhwangyh May you fix the lint issue? Thanks. ref https://github.com/sgl-project/sglang/blob/main/docs/en/contributor_guide.md |
timethink
pushed a commit
to timethink/sglang
that referenced
this pull request
Mar 9, 2025
Co-authored-by: mulin.lyh <mulin.lyh@taobao.com>
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 28, 2026
Boot 13 (4fde6e5) did not crash: it WEDGED. 5 queued, 0 running, no first token, on all three ranks, behind 239 identical refusals of one shape: sgl-project#791 FORWARDED SCHEDULE UNEXECUTABLE for rid=901a7d29…: the decision names prefix_len=0, this rank holds 7938. ROOT, and it is the second half of the exemption sgl-project#992 found. `sgl-project#791 PP ADMISSION UNIFORMITY` has two halves -- MEMBERSHIP ("admit only what the decision names") and GEOMETRY ("with exactly the prefix it named") -- and both are implemented inside the waiting-queue loop, applied strictly before `adder.add_one_req`. The chunked continuation does not travel that loop: it reaches `can_run_list` through `add_chunked_req`, ~250 lines earlier, and was therefore exempt from BOTH. sgl-project#992 closed membership. Geometry stayed open, so a NAMED continuation entered the batch carrying its own prefix while the decision named another -- and that is a SHAPE disagreement, because `prepare_for_extend` sizes the cross-stage tensor directly off `len(req.prefix_indices)`. The follower refused, correctly, every pass. The decision said 0 because PP0 had spent its `sgl-project#946 PREMISE RECOMPUTE` terminator on that rid. That is PP0's call to make and the follower's to execute; second-guessing it is what the refusal exists to prevent. FIX: apply the same `truncate_prefix_to(told)` the queue loop applies, at the same point in the same order -- immediately before the re-derivation. sgl-project#930: the helper moves `prefix_indices` and `cache_protected_len` together. sgl-project#961: the mover must be followed by the re-derivation it invalidates; in the queue loop that is `add_one_req`, here it is `add_chunked_req` on the next line. Nothing is re-derived by hand and no new helper is introduced. Logged with rid, local prefix and told, counted, so the adoption is affirmative rather than inferred from the absence of refusals. SIBLING SWEEP: the two entries into `can_run_list` under a forwarded schedule are the queue loop and this one. Both now carry both halves of uniformity. PP0 has no incoming decision (`incoming is None`), so its path is byte-identical. BOOT 13 ALSO BOUGHT: sgl-project#993's containment fired and held -- no repeat of boot 12's allocator assert. sgl-project#992 held -- no repeat of boot 11's sgl-project#801-spin. Batch lines were symmetric 5/5/5 across ranks, i.e. no rank is structurally excluded. Evidence: desk. py_compile + import smoke + wiring assertion. Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 28, 2026
…resident at all Boot 14 (cf16281) died on PP1 after 39 s: `assert self.chunked_req is None`, scheduler.py:9784 in `_get_new_batch_prefill_raw`. Third recorded fundstelle of this family after :9286 (sgl-project#951) and :9367 (sgl-project#959). ROOT, and it corrects my own attribution in the boot-14 register entry. The invariant behind that assert is held "by ARITHMETIC, not by a check" (scheduler.py's own comment). sgl-project#959 therefore gave the two `add_one_req*` mint sites an explicit `chunked_req_outstanding` check, and SKIPPED the third -- `PrefillAdder._add_scheduled_req`, the forwarded-schedule execution path -- reasoning at schedule_policy.py that it "already has its own (`carried_chunk`)". That is the guard-comment-names-the-hazard trap. `carried_chunk` answers "is THIS request the resident continuation". The invariant needs "is there a resident continuation AT ALL". It covers a request being re-announced; it does not cover a DIFFERENT named request becoming a second continuation while the first is resident. Two of three sites guarded, one not. Boot 14 is that gap on metal: the resident continuation survived `add_chunked_req`, so `chunked_req_outstanding` was True and both sibling sites correctly refused -- and this site minted anyway, on another rid the same forwarded schedule named. sgl-project#994 EXPOSED THIS, IT DID NOT CREATE IT. Boot 13 never reached the line because the sgl-project#791 geometry refusal killed every pass before a batch was built; sgl-project#994 removed that refusal. My register entry attributed the death to sgl-project#994's effect on `rem_chunk_tokens` arithmetic. That was wrong: both sibling sites were already guarded and did refuse. The arithmetic was not the hole; the missing third guard was. Withdrawn here rather than left standing. FIX: the sibling guard, at the site that lacked it. WHY A PASS REFUSAL AND NOT A REQUEST SKIP -- the danger direction, which is the whole question here. On a forwarded schedule this rank may NOT drop a named request: the upstream's hidden states for it are already on the wire, which is what this same method already raises `PPScheduleRefused` for a few lines above. Running the chunk WITHOUT announcing it is worse -- the continuation would be untracked and re-prefilled next pass, the double prefill the standing law forbids. So the disposal is the one this path already owns: refuse the PASS by name, let sgl-project#791/sgl-project#797 void and re-derive. It cannot starve: the resident continuation is consuming chunks, and when it finishes `chunked_req` is None and the schedule is executable. This is the same direction sgl-project#959 chose ("the resident continuation is never the one to give way; the fresh admission is") -- not the sgl-project#858 mid-prefill wedge, which would be clearing `scheduler.chunked_req` instead. FUTURE CHECK: `grep -c 'if self.chunked_req_outstanding:'` over schedule_policy.py is now 3 and equals the number of `new_chunked_req` writers. A fourth writer that skips it is visible as an inequality rather than as a boot death. Execution proof (speed mode, one instrument per link): `note_second_continuation_refused(req, "_add_scheduled_req")` counts it and the refusal text names itself, so boot 15 measures whether the guard fires at all -- absence of the assert alone would be green-by-absence. Evidence: desk. py_compile; local-import dominance and guard-before-raise- before-mint verified by source order in the loaded module; guarded-site count 3 == mint-site count 3. Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 29, 2026
The fix form is settled by boot 26's evidence: sender rows == received rows
with mb_id and epoch both matching, so the payload is intact and the PAIRING
is wrong, and `pp_proxy_stamp_names_pass` cannot separate the two messages.
The seqno can -- it was built for exactly this ("distinguishes two messages
for the SAME slot, which is exactly the pair a stranded leftover creates")
and no consumer has ever read it.
THIS COMMIT DOES NOT ARM IT, and that is the point.
The dangerous direction is not "a leftover gets through", it is "a VALID
message is refused". sgl-project#995's first version proved the cost on metal: a
refusal on the live path with no way onward produced 175 refusals on ONE rid
in 40 s, four batches, and a dead window. So the can-fail is built on that
side first, exactly there and not on the comfortable side.
WHAT IT MEASURES: whether the width available at the receive site agrees
with the width the downstream guard will use. If it ever disagrees on a
message that is NOT a leftover, an armed check here would kill good passes,
and that must be known BEFORE the check exists rather than after a boot dies
of it.
WHY THE COMPARISON IS TRUSTWORTHY, and this is the part that separates it
from sgl-project#994's mistake: it is NOT a second derivation of the same idea.
`ForwardBatch` is constructed with `input_ids=batch.input_ids`
(forward_batch_info.py:709), so `self.mbs[mb_id].input_ids` IS the tensor
`model_runner.forward` measures -- the same object, read at two points.
sgl-project#994 truncated a geometry that a parallel computation then re-derived
differently, and that is what made it a producer instead of a fix. Verified
in the smoke rather than assumed, because the constructor could change.
NOTHING IS REFUSED: the probe cannot raise (asserted in the smoke), the pass
still runs, and the downstream width check in `model_runner.forward` remains
the only gate. Disagreements are counted and named with both widths and the
sender's (mb_id, seq, epoch); agreements are counted so the log states the
denominator instead of leaving "no hits" ambiguous between "never disagreed"
and "never ran".
WHAT ARMING WILL NEED, stated now so the next step is not re-derived: a
DEFINED ACTION for the rejected message -- drop the leftover and receive
again -- and a defined state for the waiting receiver. Detection is half the
work; surviving is the other half, and the half sgl-project#995 skipped.
STILL OPEN, filed and not chased before the boot: WHY a leftover carrying
this slot's own identity exists at all. This change makes the mispairing
visible and will make it preventable; it does not explain the leftover.
Evidence: desk. py_compile; smoke asserting the probe body contains no
raise, and that ForwardBatch is still built from `batch.input_ids` so the
same-object claim holds. Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 29, 2026
`trace_round` is the only rid-precise instrument that separates "this round appended nothing" from "this round produced nothing" -- the two cases the void->output_ids->fill chain has to tell apart. Its own docstring says so, and the sgl-project#631 comment directly above the decode append (batch_result_processor.py:853) says the same: "A round that appends nothing and a round that produced nothing look identical in output_ids alone." It could never fire on this configuration. `armed_after` is `after_left > 0`, and `after_left` was set in exactly one place: `cutover()`. With the flip off there is no cutover, so the instrument built for this question is structurally unavailable on the config where the question now lives -- which is why nobody has measured it there. That is the third instance of one pattern in this window, and the most expensive of the three. sgl-project#994 framed "bigger pool serves worse" as flip-bound while the mechanism occurs flip-free; sgl-project#987 framed its fill divergence as "across tp_to_pp" and I measured it with flipdone=0. Those cost a wrong search each. A flip-bound INSTRUMENT costs the possibility of searching at all, and it does so silently: a zero from a never-armed probe looks exactly like a zero from an armed one. A SECOND GATE, NEVER INSTEAD OF THE FIRST. `cutover()` still sets `after_left = self.post` untouched -- taking the flip path's window away to give this one a window would be the next regression. `SGLANG_631_TRACE_OFFFLIP=N` arms N rounds at construction; unset or 0 is today's behaviour byte for byte, asserted in the smoke together with the cutover path still setting its own window afterwards. Evidence: desk. py_compile; smoke asserting the default stays disarmed (after_left 0, armed_after False), that the env arms it (6, True), and that `cutover()` afterwards still sets its own window (post=8). Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 29, 2026
#997c armed the sgl-project#631 output trace without a cutover and changed nothing, because the arming was never the binding level. Three levels, and the lowest one decides: HOST `trace_tick` is called from `_phase_flip_on_round` alone, and both of THAT function's call sites sit behind `if server_args.enable_phase_flip`. On a flip-free build the OutputTrace object is never CONSTRUCTED, `_ACTIVE_TRACE` stays None for the life of the process, and `trace_round`/ `trace_emit` return on their first line. ARMING `after_left` was set by `cutover()` only -- what #997c fixed. ENV `trace_enabled()` defaults on, no gate at all. A zero from an instrument that was never built reads exactly like a zero from one that ran. This trace is the only rid-precise instrument that separates "this round appended nothing" from "this round produced nothing", and its subject -- per-round, per-rank growth of `req.output_ids` -- is ordinary serving, not the flip. sgl-project#994 and sgl-project#996 were both framed flip-bound and both measured occurring flip-free; a flip-bound root costs one wrong search, a flip-bound INSTRUMENT costs the possibility of searching at all. A SECOND HOST, NEVER THE MOVED ONE. `trace_tick_offflip` is called from `get_next_batch_to_run` OUTSIDE every flip gate, and returns immediately when `enable_phase_flip` is set. The flip path keeps its tick, its pre-cutover ring and its post-cutover countdown byte for byte -- ticking from both hosts would double-count `pass_no` and consume the countdown at twice the rate, i.e. corrupt the flip's reading to serve a build that is not this one. OBSERVER DISCIPLINE (sgl-project#986): CADENCE, NO HIT FILTER. The round counter advances on every round the host sees; the burst opens on the counter alone. Nothing about what a burst found can move the next one, so a silent burst is evidence. DENOMINATOR AT THE OPENING, NOT AT THE FINDING. The window logs `resident=N` where it opens. resident=0 is a legitimate reading (idle instance); the ABSENCE of that line is what means the host stopped. BURSTS ARE CONSECUTIVE ROUNDS (default 4). The measurement is a per-round delta and one isolated round has none. `_fmt_delta` no longer returns "" for empty rows with a prev set -- it printed a line ending in "-- " with nothing behind it, so an armed round holding no requests and a formatting failure read identically. Same false zero, one level down. Default: SGLANG_631_TRACE_OFFFLIP_EVERY unset -> the host returns on its second line, no object built, no attribute written. Byte-identical. SWEEP, counted so the number does not inflate on its way onward. 55 textual occurrences of `enable_phase_flip` in the tree before this commit; 30 of them are distinct RUNTIME DECISION sites and all 30 were read with enclosing-function context and classified at all three levels -- 13 in scheduler.py, 5 in scheduler_pp_mixin.py, 3 in metrics_reporter.py, 2 each in model_runner.py and model_runner_kv_cache_mixin.py, 1 each in phase_flip_boot.py, phase_purity.py, corridor_admission.py, mem_ledger/engine.py and memory_pool_host.py. The other 25 are not gates: the flag's own definition and validation in server_args.py, prose in a planner/rejected.py record, comment text, and parameter/kwarg pass-through in scheduler.py and phase_flip_boot.py. Beside those, the instrument-shaped entry points of 12 phase_flip_*/corridor_*/ layout_conformance modules were enumerated and their hosts resolved. OPEN: 0 of the 30 unclassified. NOT covered by this sweep, and named rather than implied: instruments hosted on a flip gate that is not spelled `enable_phase_flip` (an `armed`, `pending` or `epoch` predicate reached from a non-flip caller) -- corridor_steering and corridor_rebalance are of that shape and were resolved by hand, but the class was not swept exhaustively. ONE confirmed member of the class among the 30: this one. The rest are either actuators (corridor_steering, corridor_rebalance -- flip-hosted, but their product is an action, not a reading) or instruments whose SUBJECT does not exist off-flip (layout conformance, phase label, seam reserve and its at-rest measurement, arena-tail derivation, bootstrap clock, seam census, KV shrink verdict). Flip-hosted is not by itself the defect; flip-hosted while measuring flip-free serving is. Evidence: desk. py_compile both files; a smoke covering the default (nothing constructed, `_ACTIVE_TRACE` None, round attribute unwritten), the flip build (host declines, builds nothing, counter unmoved), the off-flip build (object constructed on round 1, burst arms on cadence, `trace_round`/`trace_emit` go live), the cadence against an instance with nothing to find (opens at 3/6/9/12), and the empty-rows denominator -- 15/15, with a can-fail proof (neutralising the host turns it red). An AST walk over scheduler.py confirms the single new call site carries NO enclosing phase-flip gate, which is the host-level check whose absence cost the #997c cycle. ruff: 42/160 findings before and after on the two files, no new one; both files were already unformatted at HEAD. Belegstufe: DESK-BEWIESEN. Not boot-proven, no metal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Support models from www.modelscope.cn, running server.