Skip to content

[Fix] sampling_params and Runtime bugs for some cases. - #1158

Closed
gryffindor-rr wants to merge 17 commits into
sgl-project:mainfrom
gryffindor-rr:main
Closed

gryffindor-rr wants to merge 17 commits into
sgl-project:mainfrom
gryffindor-rr:main

Conversation

@gryffindor-rr

Copy link
Copy Markdown
Contributor

Motivation

Modification

bug 1:
data = json.dumps(sampling_params) and then SamplingPramas(json.loads(data)), will report no attribute 'stop_strs', modify the position argument name consistent to self variable.

bug 2:
sampling_params { "n": 3,...} will report error, 'Do not support stream for batch mode'
Fix runtime to set stream when n>1.

Tested by examples/usage/async_io.py

Checklist

  • Before submitting a PR for review, make sure it has passed verification in your local development environment at least.
  • 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.

@vhain

vhain commented Aug 20, 2024

Copy link
Copy Markdown
Contributor

stream: true with n > 1 could be supported with #1134

@gryffindor-rr

Copy link
Copy Markdown
Contributor Author

or maybe another esier way to support json.dumps for sampling_params: https://github.com/gryffindor-rr/sglang/commit/de4be50de34d741dea550d6e6f10c8bcd966bbd4

@merrymercy merrymercy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that stop is in the public API, we do not want to change it to break existing apps.

Your new commit is better. Can you submit a PR for that?

if isinstance(sampling_params, dict) and sampling_params.get("n", 1) > 1:
stream = False
else:
stream = True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this as it is supported in d847681

@merrymercy

Copy link
Copy Markdown
Contributor

@merrymercy merrymercy closed this Aug 20, 2024
efschu pushed a commit to efschu/htsglang that referenced this pull request Sep 3, 2026
…sgl-project#939 census sees the pp_to_tp wave; one line for a hung warmup; rate-limit the sgl-project#1073 resume line

Boot weg1b3 (/spinning/evidence-665-f1/boot_855_weg1b3_6980c75eac_0902_234752.log,
200766 lines) served 0 of 6 driver requests and was ended by its own
watchdog/SIGQUIT chain at 00:06:52. Four in-tree amplifiers/instruments from
that boot, each with the line that measured it.

1. sgl-project#833 STALLED arm misread an advancing chunked prefill (phase_policy.py,
   scheduler.py). log:100502 armed "decode bundle STALLED, not draining: 1 of 1
   req still decoding and the set has not shrunk for 11.4s" against rid
   679e4568, whose ADMIT prefix ladder was strictly monotone 4096 -> 20480 in
   that window (one 4096 chunk every ~3 s). A chunk-prefilled request holds
   running_bs FLAT by construction and is not a seam cohort member, so neither
   existing progress axis moved. The clock now credits the SAME request's
   computed prefix growing; a different rid with a larger prefix is admission
   refilling the bundle and still reads as non-progress, so sgl-project#833's convergent
   binding is untouched. The 70 s cutover that arm bought retracted the request
   it was building (sgl-project#939 worst=79931 two lines later).
   Red-first: test/registered/unit/managers/test_1159_chunked_prefill_is_not_a_stall.py
   (4 tests, red before the fix on TypeError, then on the arm itself); mutant
   (drop the rid comparison) -> test_a_different_request_is_refill_not_progress
   FAILED "11.4 != 0.0".

2. sgl-project#939 census blind for pp_to_tp waves (scheduler.py). The census's only
   writer (schedule_batch.py prepare_for_extend, prior == 0) gates on
   SEAM_READMIT_ATTR, and _get_new_batch_prefill_raw spent that one-shot stamp
   ~240 lines BEFORE calling prepare_for_extend. On a transport_only round --
   the seam wave, the census's own population -- every request reached the
   writer already cleared. Measured: 679e4568 lost 84027 tok at 23:56:18 and
   8f31846b lost 13225 at 23:59:54, neither scored, while all six census lines
   in the boot belong to the 23:57:48 tp_to_pp wave. The spend moves AFTER
   prepare_for_extend: still one-shot, still the same pass, still before the
   function returns; nothing between the two sites reads the attr or returns.
   sgl-project#1154 first-of-wave emission untouched.
   Red-first: test/registered/unit/mem_cache/test_1159_census_sees_the_wave.py
   (5 tests; red before the fix: "the seam stamp is cleared at
   scheduler.py:11729, BEFORE the sgl-project#939 census reads it"); mutant (move the
   spend back) -> red at scheduler.py:11716.

3. sgl-project#1073 RESUMING line rate-limited (barlink_liveness.py). 82,350 of the boot's
   200,766 lines (41.0 %) at ~97 lines/s/rank from 23:59:54: a PERSISTENT
   condition reported at the watchdog's tick rate. First line kept verbatim,
   later ones counted, the next emitted line names the count
   (RESUME_LINE_MIN_INTERVAL_S = 10.0, per rank; a closed window ends the
   episode). Behaviour unchanged -- only the logging is limited.
   Red-first: test/registered/unit/distributed/test_1159_resume_line_rate_limit.py
   (4 tests; red before the fix "3 != 1"); mutant (elif True) -> 2 red.

4. #1033c warmup gets a REPORTING bound (scheduler.py, environ.py). The window
   opened at 23:59:54 on PP0 and PP2, PP1 never entered it, and no WARMUP done
   / build window CLOSE ever followed; the only evidence the boot produced was
   the sgl-project#1073 spam above. One named line from a timer thread,
   SGLANG_CUTOVER_WARMUP_OPEN_WARN_S (default 120 s, ~190x the longest honest
   warmup in that boot: 28-629 ms). It does NOT stop the group, close the
   window or redesign the wrapper -- the group stop for the one-sided cutover
   is sgl-project#1158, and the docstring says so.
   Matched check (hermetic, no GPU): arm A a 1.0 s forward against a 0.3 s
   bound emits exactly 1 OPEN line; arm B a 10 ms forward emits none. Mutant
   (never arm the timer) -> "expected 1 OPEN line, got 0".

docs/dev/WEG1_1068.md gains section 7 "Boot 3 (weg1b3) findings": R1 store read
serves nothing (sgl-project#1157), R2 rank-divergent TP admission (sgl-project#1158), R3 amplifiers
and R4 instruments (sgl-project#1159), pointing at
/spinning/gpu-arb/HANDOVER_WEG1B3_0903.md.

Evidence tier: DESK-PROVEN. No boot, no GPU. ruff parity vs HEAD on every
touched file (scheduler.py 102 -> 102 pre-existing, others clean); the three
new test files are ruff-clean and ruff-formatted.
efschu pushed a commit to efschu/htsglang that referenced this pull request Sep 3, 2026
…llot digest mismatch is a group STOP; the cold-build opener honours the cap

Boot weg1b3 @ 6980c75 (log /spinning/evidence-665-f1/boot_855_weg1b3_6980c75eac_0902_234752.log):
a /health_generate probe reached all three ranks at 23:54:18; PP0 was busy and
dropped it in its dispatch loop, PP1/PP2 took the sgl-project#631 row-authority branch of
the same loop and enqueued it (queue=6 vs 7, +1 through five seams). The #791b
ballot saw the disagreement on every TP pass from 23:56:17 (18 mismatch lines,
cadence 1..32, 0 restored) and fell back to the rank-local prefetch verdict,
until at 23:59:54 the verdicts split: PP0/PP2 admitted 8f31846b and entered the
forward, PP1 declined and sat in the request broadcast. The opener-side cold
build window (x40, uncapped) held the hung forward until the 300 s watchdog.

F2a ONE DISPOSAL, AT THE ORIGIN, BEFORE RELAY (upstream-minimal).
  request_receiver.recv_requests: on the request origin only, health-check
  requests meet the existing idle gate BEFORE the policy hook, the TP
  broadcast and the PP chain forward (busy -> ipc answered from the origin +
  dropped; idle -> kept and relayed like any request). The rank-conditional
  block in scheduler.process_input_requests (upstream idle gate + the sgl-project#631
  follower-enqueue special case, 8da4ec5) is deleted: every rank
  dispatches the identical list and waiting_queue stays replicated. Drop
  line: 'sgl-project#1158 HEALTH-CHECK dropped at origin before broadcast rid=%s busy
  queue=%d running=%d' (the gate's own reading). The PP chain forward
  (_pp_forward_and_process_input_requests) sends the list recv_requests
  RETURNED, and the DP-attention work/control split runs inside
  _broadcast_reqs_across_ranks after the disposal, so no second path can
  deliver a probe the origin dropped.

F2b A BALLOT DIGEST MISMATCH IS A STOP (raenge-nie-uneins).
  prefetch_ballot.unpack_prefetch_ballot raises PrefetchBallotDigestMismatch
  (RuntimeError) on min != max: '#791b PREFETCH-BALLOT DIGEST MISMATCH STOP
  rank=%d digest=%d group_min=%d group_max=%d queue_len=%d head=[%s]'. The
  (x, -x) pair rides a MIN all_reduce, so every rank of the reduce holds the
  same min and max and raises on the SAME pass. The scheduler's void ballot,
  the fallback to the rank-local verdict, the streak counters and the
  geometric cadence line are deleted from _update_uniform_pool_budget; a
  missing ballot slice in the TP loop is its own '#791b PREFETCH-BALLOT
  LAYOUT STOP'. None stays only for the single-rank / PP-loop callers of
  prefetch_done_under_ballot. Group stop = the existing
  run_scheduler_process except -> SIGQUIT -> kill_process_tree
  (_update_uniform_pool_budget is called from get_next_batch_to_run with no
  except in between); no new collective. advance_mismatch_streak stays for
  the sgl-project#823 head-congruence COUNT/ORDER degradation counters (its one
  remaining caller, _note_tp_head_degradation) and says so.

F2c THE OPENER HONOURS THE CAP TOO.
  jit_cold_build.capped_cold_build_deadline(base, cap) = min(base * mult,
  base + cap) is the ONE formula for both readers: barlink_liveness.
  wait_timeout_s (host seconds) and jit_cold_build.resolve_timeout_cycles
  (device cycles, cap expressed at the documented 2 GHz nominal clock). cap =
  barlink_build_window.build_cap_s(), the same SGLANG_BARLINK_BUILD_WINDOW_
  CAP_S the peers honour (60 s on this rig); cap 0 = no extension on both.

F2d SIBLINGS.
  flush_cache on rank-local is_fully_idle: judged and left, documented in
  place -- with the one disposal at the origin the queues are replicated
  again and the predicate reads the same on every rank. tp_head_congruence
  ORDER arm: the 'except Exception -> SOURCE_RANK_LOCAL' degradation in
  _apply_uniform_head_order was one block and is now the named
  'sgl-project#823 HEAD-ORDER APPLY STOP' (re-raised from the cause); it propagates
  through _get_new_batch_prefill_raw on the same path as the sgl-project#1153/sgl-project#1157
  stops. _drain_prefetch_progress's replicated-queue premise holds again by
  construction (no edit).

Tests (hermetic, CUDA_VISIBLE_DEVICES="" PYTHONPATH=<tree>/python):
  NEW test/registered/unit/managers/test_1158_one_disposal_and_stop.py
  (2-rank fake of recv_requests over a fake broadcast wire; AST proof that
  is_health_check_generate_req is consulted at exactly one disposal site and
  pp_rank no longer appears in process_input_requests; digest-mismatch raise
  with both digests / none on agreement; head-order STOP) and
  test/registered/unit/distributed/test_1158_opener_honours_cap.py
  (window open, mult 40, base b: host b+cap, device b+cap cycles, non-binding
  cap untouched, cap 0 = no extension, one formula). INVERTED with withdrawal
  docstrings: test_pp_prefetch_ballot_791b.py
  test_a_divergent_queue_head_voids_the_ballot_loudly ->
  ..._stops_every_rank; test_prefetch_ballot_divergence_823.py
  TheLatchIsGoneAndTheRecoveryEdgeExists (5 probes turned around) and
  TheFallBackBehaviourIsUnchanged (None = no ballot taken).
  Red-first on the parent 8a7dd38 (tests run from this tree against the
  parent's code, never copied in): one_disposal 16 failed / 4 passed,
  opener 4 failed / 1 passed, 791b 1 failed / 8 passed, 823 4 failed / 16
  passed; the survivors pin unchanged behaviour (wrong-width None, agreement
  verdict, ballot-less local verdict, disabled gate, non-binding cap).
  Fixed tree: 54 passed.
  Mutants (each restored, all red): keep the probe when busy 2F; skip the
  disposal at the origin 3F; raise only when group_min == 0 4F; host reader
  uncapped 2F; device reader uncapped 3F; except degrades to
  SOURCE_RANK_LOCAL 2F.
  Bounded suite test/registered/unit/managers (+ the new distributed file on
  the fixed side only), pytest -q -rf -n 4 --dist loadscope:
    parent 8a7dd38 (scratch worktree, new files excluded): 246 failed / 4648 passed / 18 skipped / 8 errors (280.69 s)
    fixed: 246 failed / 4673 passed / 18 skipped / 8 errors (274.84 s)
    comm over the FAILED ids: 242 unique FAILED ids on each side, 0 only-on-parent, 0 only-on-fixed
    (-rf lists FAILED only; the 8 errors are collection errors on both sides,
    not listed by id)
  Harness stand-in: test_collective_family_siblings_610.py BudgetHarness
  gains `ps = SimpleNamespace(tp_rank=...)` because the reduce now reads
  `self.ps.tp_rank` for the STOP line (the file's own drift guard caught it,
  eighth drift of that harness); 11 passed.
  ruff 0.15.1 on the touched files: parent 103 errors / fixed 103 errors
  (parity; the two new files: 0).

Evidence tier: DESK-PROVEN. Boot 4 proof lines: 'sgl-project#1158 HEALTH-CHECK dropped
at origin' on PP0 only; every 'sgl-project#788 PP-ADMISSION ... queue=' triple equal
across ranks after each health probe; grep -c '#969C READMIT-PREFETCH.*rid=
HEALTH' = 0; 'Ballot void for this pass' = 0; every 'PHASE-FLIP SEAM
RE-ADMISSION' triple identical; every '#1033c CUTOVER FORWARD WARMUP begin'
matched by a done or a named abort within base+60 s; 'watchdog timeout' = 0.

This commit audits and completes the partial staged work of a previous
sgl-project#1158 builder (r2_partial_0903.patch); the drop line now prints the gate's
own reading, and the opener test collects on a tree without the fix.
efschu pushed a commit to efschu/htsglang that referenced this pull request Sep 3, 2026
…xecute the health-check gate body in a test, name the allowance coupling

1. English-code standard: replaced the German noun 'frist' with 'deadline'
   in the four remaining docstrings/comments (barlink_liveness.py :179,
   :759, :791; jit_cold_build.py :158). grep -rn frist python/sglang/srt
   now prints nothing.

2. Test gap named by review: Scheduler._health_check_gate had no test.
   Added HealthCheckGateReturnsTheOriginVerdict to
   test/registered/unit/managers/test_1158_one_disposal_and_stop.py,
   calling the method on a types.SimpleNamespace stand-in (is_fully_idle,
   waiting_queue, running_batch.reqs) with is_fully_idle True and False.
   Mutant proof: inverting the boolean in the method body
   (bool(not self.is_fully_idle(...))) turned exactly the 2 new tests red
   (2 failed, 20 passed, all failures confined to the new class); source
   restored (git diff --stat over scheduler.py empty afterward); full
   file green again at 22 passed (20 pre-existing + 2 new), 0 failed.

3. Docstring-only addition to _store_witness_allowance in
   phase_purity.py naming the coupling review found: an unset mamba
   anchor interval today, and what breaks (a capped probe can land on
   the deepest anchor boundary, a shortfall up to interval-1 becomes a
   contradiction by construction) plus the required fix if that form
   ever ships. No behaviour change.

ruff check on all four touched files: All checks passed! before and
after (0 issues both times).
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.

3 participants