Revert "Temporary fix invalid sample results" by hnyls2002 · Pull Request #673 · sgl-project/sglang

hnyls2002 · 2024-07-20T09:05:17Z

Reverts #668

This reverts commit 8f4b155.

* [feat] Use einsum matmul op in MLA matrix absorb

…oes (gated, red-first) THE ABORT IS THE EXIT, not the work. "terminate called without an active exception" after a CLEAN drain is the precise signature of a std::thread destructor running while the thread is still joinable -- "without an active exception" rules out the throw-in-thread variant. ROOT CANDIDATE 1, provable by inspection and indicted by torch itself: * distributed/parallel_state.py:3835 defines destroy_distributed_environment and :3845 defines cleanup_dist_env_and_memory. Across ALL of python/sglang/srt, NEITHER HAS A SINGLE CALLER (the only callers in the tree are in the unrelated multimodal_gen subtree, which has its own copy). * torch reports the omission in the specimen itself: "ProcessGroupNCCL.cpp:1575 WARNING: destroy_process_group() was not called before program exit". * ProcessGroupNCCL's watchdog and HeartbeatMonitor are C++ threads joined by the group's DESTRUCTOR -- and HeartbeatMonitor::runLoop() appears BY NAME in specimens 2 and 3. * The scheduler's graceful teardown (scheduler.py finally) runs close_regime_trace -> _shutdown_fpm -> release_host_resources and returns. No distributed teardown anywhere. So at exit the groups are alive, their threads joinable, and whichever destructor runs first calls std::terminate. That explains every property of the family: after a clean drain, no active exception, no Python frame (C++, and in specimens 2-3 the Python main thread is provably asleep elsewhere), chronic across boots (unconditional), intermittent per process (which destructor wins). FIX: managers/scheduler_teardown.py release_distributed(), called from the finally in run_scheduler_process. Graceful path ONLY (destroying a group synchronises; on the exception path the GPU may be wedged and a teardown that HANGS is worse than the abort it prevents, which at least ends the process -- the same guard release_host_resources already carries), never raises (it runs in a finally during shutdown, where raising would replace a clean exit with a traceback or mask the original failure), idempotent. DEFAULT OFF, and not out of timidity: GroupCoordinator.destroy (parallel_state.py:2501-2508) closes barlink_comm BEFORE destroying the groups, and barlink owns a POSIX shm segment plus the device-mapped abort word that spinning kernels read (barlink_liveness.py:383,449). That is sgl-project#722's live machinery in another lane. No barlink file was touched; arming this changes WHEN that memory is unlinked relative to those kernels, so the lane that owns barlink decides when to arm it. REPORTED AS AN OVERLAP, not edited. Red-first: test/registered/unit/managers/test_scheduler_teardown_673.py (10) pins that the teardown runs when armed, in the right ORDER (model-parallel before the world, since the former is built on the latter), never on the exception path, never by default, never raises, attempts both destroys even when the first fails, and reports what succeeded. TestTheOmissionIsReal fails if anyone ever adds a caller for cleanup_dist_env_and_memory -- i.e. it tells the next reader that sgl-project#673's premise moved and the default should be revisited. A test-construction bug of my own, found by running: the first version patched sys.modules only, but `from sglang.srt.distributed import parallel_state` resolves the package ATTRIBUTE, so the REAL destroy ran inside the test process and quietly succeeded. Both are patched now, so the test cannot pass by accident. DIAGNOSIS: evidence-665-f1/DIAG_673_TEARDOWN_ABORT.md continued in place (the qwen lane's three specimens are intact above the continuation). It now carries the specimen reading, both root candidates, the completed background-thread inventory, the revised ranking, and the explicit statement that this fix addresses candidate 1 ONLY -- four other never-stopped C++-bodied threads are named with their file:line and left to their owners, deliberately, so that a live A/B of this flag points at one remaining hypothesis rather than five. Regression, same env: unit/managers 4 failed / 2145 passed -> 4 failed / 2155 passed (+10, the new file); scheduler/test_phase_flip_runtime.py 67 passed. scheduler.py diff: 11 insertions, 0 deletions. Live check when a window opens (boot-gated, one boot): arm the flag and grep the shutdown for the abort and for the "destroy_process_group() was not called" warning. Both should disappear together. CAN-FAIL PROOF (mutation applied, suite re-run, reverted): U1 the teardown never runs (fix removed) -> 5 failures incl. test_the_graceful_path_destroys_the_groups U2 the graceful guard removed (would run on the exception path) -> test_the_exception_path_never_destroys, and ONLY that one U3 the gate removed (would run by default, colliding with sgl-project#722) -> test_off_by_default, test_a_scheduler_without_server_args_is_a_no_op U4 destroy order swapped (world torn down before model-parallel) -> test_model_parallel_is_destroyed_before_the_world, +2 U5 a failing destroy escapes the finally -> test_a_failing_destroy_never_raises, test_a_partial_failure_still_reports_what_succeeded Restored tree re-verified green after every mutation (10 passed).

… sibling sweep server_args.py imported ONE INTEGER -- `from ...fla.chunk_delta_h import CHUNK_SIZE` -- and thereby pulled torch, triton and the whole FLA kernel chain into every process that touches ServerArgs: launcher, tokenizer manager, detokenizer. The cost is not only import time: fla/utils.py:283 runs get_available_device() at MODULE SCOPE, reaching torch.cuda.is_available() and the triton driver, so processes that must never hold a CUDA context were touching the driver while parsing arguments -- the sgl-project#237/sgl-project#403 second-context family. ATTRIBUTION FIRST, measured with a fresh-subprocess sys.meta_path tracer, because the obvious probe lies. An earlier crude check ('.fla' in module name) reported FLA everywhere -- it was matching torch.nn.modules.flatten. With a precise prefix match: * sglang's FLA chain: pulled by server_args.py:61 and by NOTHING else (a bare `import sglang` does not pull it) -> removable, and now removed; * triton: utils/common.py:89, module level; * torch: utils/common.py:87, module level; * torchvision: from `transformers`, via sglang/__init__.py:29,31. So "make server_args import neither torch nor triton nor fla" is achievable TODAY only for fla; the other two are already in sys.modules before server_args is reached, and no change to server_args can alter that. FIX: FLA_CHUNK_SIZE = 64 defined in server_args (the upstream value is a plain literal, fla/chunk_delta_h.py:23). PINS (test_server_args_import_weight_673.py, 4): * FLA absent after importing server_args, checked in a FRESH SUBPROCESS -- in-process sys.modules is already polluted by the test runner, so an in-process assertion would pass or fail for unrelated reasons. RED-FIRST CONFIRMED: restoring the old import fails exactly this test. * the inlined constant checked against FLA's authoritative CHUNK_SIZE, so the duplicate cannot drift silently -- inlining trades an import for a copy, and an unpinned copy is worse than the import was. * an ATTRIBUTION pin asserting today's UNFIXED state (torch and triton ARE present after a bare `import sglang`), which fails when the package root gets lighter -- that failure is the signal to tighten the first pin. Filing a green test against a red world would have been the dishonest alternative. * a file:line pin on utils/common.py:87/:89 so the next reader does not re-derive the ownership. SIBLING SWEEP of the always-loaded path (sglang/__init__ -> srt/utils/__init__ -> utils/common.py, which every process loads). Fixed only the trivial ones, meaning single use inside a function with no behaviour change: * torchvision.io.decode_jpeg -> deferred into its one CUDA-only call site. Honestly: torchvision still arrives transitively via transformers, so this win is LATENT until the root's HF patch is addressed. * starlette.routing.Mount -> deferred into the metrics-app builder; this one does disappear from the import set today. Filed with blockers named, not fixed: triton (blocked by the MODULE-LEVEL monkeypatch at utils/common.py:3717, setattr(triton, "next_power_of_2", ...), which must run before any kernel reads it -- deferring the import means deciding where that patch goes, which is its owner's call); transformers at the package root; psutil/torch.nn/PIL/requests/packaging (multi-site); torch (structural). Measured effect: importing sglang.srt.server_args goes 5161 -> 5134 modules. Small, and that is the honest headline -- the package ROOT dominates, and until it is lazy no downstream change makes any process torch-free. Regression, same env: unit/server_args 1 failed / 639 passed -> 1 failed / 643 passed (+4, the new file); unit/managers 4 failed / 2155 passed unchanged; unit/mem_cache 940 failed / 938 passed unchanged. Pre-existing failures are GPU-required tests under CVD="". Diagnosis appended to evidence-665-f1/DIAG_673_TEARDOWN_ABORT.md. CAN-FAIL PROOF (mutation applied, suite re-run, reverted): V1 the FLA import restored (the defect itself) -> test_importing_server_args_does_not_import_fla, and ONLY that one V2 the inlined constant drifts (64 -> 128) -> test_chunk_size_matches_the_authoritative_value, and ONLY that one (the containment pin doing exactly its job) V3 utils/common stops importing triton at module level -> test_the_owner_of_the_torch_and_triton_pull_is_named (the attribution pin noticing that ownership moved -- which is the point: it fires when the sibling fix lands, so the FLA pin can then be tightened) Restored tree re-verified green after every mutation (4 passed).

…riton's own import; defer the video probe THE PATCH. utils/common.py did `import triton` at module scope purely to `setattr(triton, "next_power_of_2", next_power_of_2)`. utils/common is reached by the package root, so the tokenizer manager and the detokenizer -- processes that must never touch CUDA (sgl-project#237/sgl-project#403) -- loaded a GPU kernel compiler to install a two-line integer helper they never call. Neither obvious option was available. DELETING the patch: 235 call sites read triton.next_power_of_2. MOVING it next to the first consumer: with 235 readers there is no first consumer, and a patch that races its readers is a silent numerics/perf landmine rather than a crash. So it is applied BY THE IMPORT. utils/triton_patch.py installs a meta-path finder that claims `triton`, resolves the real spec through the rest of the path, and wraps its loader so the attribute is set after the module body runs and BEFORE `import triton` returns. The ordering holds by construction, not by discipline: a reader must import triton to read the attribute, and the patch is installed inside that import. install() also patches a triton that is already in sys.modules, since a module already imported cannot be patched by a future import. Arming costs one object on sys.meta_path and no triton. WHAT THAT ALONE BOUGHT: NOTHING, measured. +1 module, RSS unchanged -- because triton still arrived through torchcodec -> torch._dynamo. Reported rather than dressed up, and it is why the sweep continued into the next link. THE ACTUAL WIN was a second import-time CAPABILITY PROBE, the same antipattern as FLA's device probe: video_decoder.py did `try: from torchcodec.decoders import VideoDecoder` at module scope to set `_BACKEND`. Deferred behind a cached backend() with a PEP 562 module __getattr__ so existing readers of `video_decoder._BACKEND` keep working. Measured, per process type, before vs after (module count / max RSS): tokenizer_manager 6780 / 980 MB -> 6760 / 952 MB (-28 MB) detokenizer_manager 6784 / 982 MB -> 6764 / 952 MB (-30 MB) utils.common alone 4340 / 809 MB -> 4320 / 784 MB (-25 MB) FILED, NOT CHASED (as instructed): triton is STILL present in every process, now via transformers -> torch._dynamo -> triton (transformers/masking_utils.py:38), reached from the package root's HF patch at sglang/__init__.py:29,31. Plain `import torch` does NOT pull triton (1104 modules), so this is transformers' chain, not torch's. A triton-free text process needs the ROOT ticket -- making sglang/__init__ lazy -- and nothing downstream of it can deliver that. LINT CAUGHT REAL BREAKAGE I INTRODUCED, worth recording because no test covers it: removing the module-level probe also removed the `VideoDecoder` name that three call sites used (video_decoder.py:91,97,176 -> NameError at runtime) and left one `_BACKEND` read behind. All four repaired with local imports. The video path has no hermetic test, so ruff was the only thing between that and a silent runtime break. Tests (12 new, hermetic, all in FRESH SUBPROCESSES because this process has triton long since imported and patched): * test_triton_patch_ordering_673.py -- arming does not import triton (the hook module loaded BY FILE PATH, since importing it normally would execute the package root and measure that instead); a reader that imports triton and reads the attribute immediately always sees the override; the CAN-FAIL for that guarantee (unarmed, triton keeps its own); retroactive patching; idempotent install; the real boot path ends up patched. * the ladder rung tightened rather than deleted: the old pin asserted utils/common imports BOTH torch and triton at module scope; triton no longer does, so the pin now asserts torch remains and triton is gone, plus the torchcodec absence and that the deferred probe still answers. Regression, same env: unit/server_args 1 failed / 643 passed -> 1 failed / 655 passed (+12); unit/managers 4 failed / 2155 passed unchanged; unit/mem_cache 940 failed / 938 passed unchanged. CAN-FAIL PROOF (mutation applied, suite re-run, reverted): W1 the hook never patches (loader wrapper made a pass-through) -> test_a_reader_importing_triton_cannot_observe_it_unpatched, test_the_real_boot_path_ends_up_patched W2 install() forgets an already-imported triton -> test_triton_imported_first_is_patched_retroactively, and ONLY that one W3 arming imports triton eagerly again (the original defect) -> test_arming_the_hook_does_not_import_triton, and ONLY that one W4 common.py imports triton at module scope again -> test_common_no_longer_imports_triton_at_module_scope, and ONLY that one W5 the torchcodec probe returns to module scope -> test_torchcodec_is_no_longer_pulled_by_common, and ONLY that one Restored tree re-verified green after every mutation (12 passed). W1 FOUND A WEAK TEST, and it is worth recording. On the first pass W1 killed only the boot-path test: the headline ordering test imported the hook normally, which executes the package root, which pulls transformers and therefore triton -- so install() patched it RETROACTIVELY and the loader wrapper was never exercised. The test passed for the wrong reason. It now loads the hook BY FILE PATH and asserts triton is absent before importing it, so it exercises the wrapper it exists to prove; W1 kills it too.

…em at the package root THE ROOT TICKET I FILED LAST PASS. sglang/__init__.py called apply_all() on the transformers compatibility patches at import time, which imported transformers in EVERY process -- and transformers reaches torch._dynamo (transformers/masking_utils.py), which imports triton. So a process that merely imported sglang loaded a graph compiler and a GPU kernel compiler. On a swapless box that is host RAM (sgl-project#721 family), not cosmetics. THE FIX. The root now ARMS the patches: a post-import hook runs apply_all() inside transformers' own import. The ordering guarantee is unchanged and if anything stronger -- a caller must import transformers to use it, and the patches land before that import returns -- while a process that never imports transformers never pays, and never needed the patches, since they only touch transformers internals. apply_all() had exactly ONE caller (the root), so nothing else changes. MECHANISM SHARED, NOT DUPLICATED. utils/post_import_hook.py is the generic "run a callback when module X finishes importing, without importing X" primitive, extracted from sgl-project#673's triton hook; triton_patch.py now delegates to it and keeps its public API. The extraction was safe to do because that hook already carried 12 tests and 5 mutation proofs. MEASURED, fresh subprocesses, before -> after (modules / max RSS): import sglang 4320 / 783 MB -> 1897 / 611 MB (-2423 mod, -172 MB) server_args 5114 / 826 -> 5115 / 825 tokenizer_manager 6760 / 962 -> 6761 / 953 detokenizer_manager 6764 / 953 -> 6765 / 953 http_server 7325 / 986 -> 7326 / 982 scheduler 6991 / 965 -> 6992 / 965 HONEST SCOPE, INCLUDING THE ZEROS. The win lands on processes that import sglang WITHOUT touching a tokenizer. The tokenizer and detokenizer managers are UNCHANGED -- and that is not a shortfall of the fix: both import hf_transformers_utils themselves, because tokenising is what they do, so transformers is a genuine dependency there rather than a root artefact. The ticket's premise (that these two process types would get lighter) was wrong, established by inspection before building and confirmed by measurement after. Everything else is +1 module (the hook) and flat RSS, which is exactly the requirement for model processes: byte-identical behaviour, and the scheduler still imports triton eagerly, deliberately unoptimised. Tests (8 new, hermetic, fresh subprocesses): a bare `import sglang` pulls neither transformers nor triton; the root arms the hook; importing transformers applies the patches, including via a SUBMODULE import (a consumer cannot slip in through transformers.utils); the can-fail (with the hook uninstalled the patches do NOT apply, so the ordering tests prove something); and the scope pins -- the text managers still import transformers themselves, and a frontend process stays under 3000 modules. LADDER RUNG CLIMBED, as designed: the previous rung asserted that a bare `import sglang` pulled BOTH torch and triton (the honest state then). Triton is now gone, so the rung asserts ['torch'] -- torch remains structural in utils/common.py. The rung failing is what told me to climb it. Two of my own tests needed repair after the shared-hook extraction, both recorded: the idempotence pin counted _PostImportFinder objects and now had to filter by target name (the transformers hook is armed too), and the isolated ordering probes had to load post_import_hook by file path rather than triton_patch, which is no longer stdlib-only now that it delegates. The guarantee lives in the generic hook, so proving it there is the more correct place anyway. Regression, same env: unit/server_args 1 failed / 655 passed -> 1 failed / 663 passed (+8); unit/managers 4 failed / 2155 passed unchanged; unit/mem_cache 940 failed / 938 passed unchanged. Frontend smoke: sglang.function/gen/user/ assistant intact, version resolves, patches applied after transformers import. CAN-FAIL PROOF (mutation applied, suite re-run, reverted): X1 the root applies eagerly again (the defect itself) -> test_a_bare_import_sglang_pulls_neither_transformers_nor_triton, test_a_frontend_process_stays_light, test_the_root_arms_the_hook X2 arm() is a no-op (patches never land) -> the three ordering pins + test_the_root_arms_the_hook X3 the hook callback is dropped (import runs, patch does not) -> test_importing_transformers_applies_the_patches, test_the_patches_land_before_the_import_statement_returns, test_a_submodule_import_also_triggers_the_patches X4 an already-imported module is not patched on install -> caught by test_triton_imported_first_is_patched_retroactively in test_triton_patch_ordering_673.py, NOT by this file. Recorded rather than papered over: the retroactive path of the SHARED hook is covered once, on the triton side, and the transformers side has no equivalent pin because arming happens at the package root before transformers can already be imported. Restored tree re-verified green after every mutation (8 passed here, 12 there).

…bort read BACK The blocker named in bdbbaf4 is cleared, and the stop caller is wired. THE BLINDNESS, restated because the fix is shaped by it. Since sgl-project#517 phase 2 the watchdog is the only reader of a BarlinkDeviceTransport's abort word: _arm_status_poll (barlink_device.py:1440) latches _abort_poll_active ONCE at bring-up (:1478), the latch is one-way, check_aborted (:1540) short-circuits on it, and _abort_code_seen is written only by poll_status_word (:1504) from the watchdog thread. Stopping the watchdog therefore left the word unread by anyone and check_aborted answered "not aborted" forever. STEP 1 -- rearm_inline_reads() (barlink_abort_gate). Stopping the watchdog now clears _abort_poll_active on every registered transport, so check_aborted falls back to the pre-sgl-project#517 in-line device read. This is the degradation should_poll_status already declares intended -- "the guard degrades to the sgl-project#517-phase-1 behaviour rather than to blindness" -- which until now was true only at bring-up, because that is where it is evaluated. The re-arm runs BEFORE the join, so there is no window in which the reader is gone while the latch still claims it reads. Pinned by a test that has the worker record the latch at the moment it observes the stop event. ABORT-WORD SEMANTICS UNTOUCHED: the word stays sticky, nothing writes it, nothing clears a code. Only WHO READS IT changes, back to the reader it had before sgl-project#517 phase 2. The in-line read it restores is the pre-sgl-project#517 per-collective device read, so its cost is paid only AFTER a stop -- during teardown, never during serving. SCOPE CORRECTION to my own bdbbaf4 wording ("every device transport"): the blindness is the barlink_device family only. BarlinkBar1Transport.check_aborted (barlink_bar1.py:5122) never consults the latch -- it always reads through _read_status_for_check (:4889) -- so bar1 was never blind; there the latch only short-circuits the watchdog's own poll. The re-arm covers both anyway, and a test pins the asymmetry so a future edit cannot quietly make bar1 depend on it. STEP 2 -- PeerWatchdog.stop, sibling shape. It used to do `thread, self._thread = self._thread, None` BEFORE the join, so a timed-out join left a live thread with no record and a second stop reported success for a thread it had abandoned. Now: join first, clear the handle only on a real join, keep it and log a loud WARNING on timeout, return "joined"/"detached"/"already stopped". DEADLINE 0.25 s, DERIVED NOT COPIED. _run waits poll_interval_s() (10 ms default) between passes, so a cooperative exit lands within about one tick plus one poll pass. 250 ms is ~25 ticks -- ample here, and 8x tighter than the 2 s the kvso and lane siblings use, which wait on a host tier write and a CUDA kernel launch respectively. STEP 3 -- the caller. barlink_liveness.stop_watchdog() is the counterpart ensure_watchdog never had. scheduler_teardown.release_barlink_watchdog() is ALWAYS-STOP with no gate: the thread leaks on every boot with barlink liveness on, whether or not the destroy is armed, so the destroy's flag must not leak into it. Ordering is enforced BY CONSTRUCTION inside release_distributed, which stops and joins the watchdog before it touches parallel_state; the scheduler also calls it explicitly beforehand, but that call order is one careless edit from inverting. NOT TOUCHED, as instructed: destroy gating, barlink_comm close order, shm unlink, abort-word semantics. No transport close path was edited. Tests, hermetic (CUDA_VISIBLE_DEVICES=""), 24 new: test/registered/unit/distributed/test_barlink_watchdog_rearm_673.py RED-FIRST: 23 failed / 1 passed before the fix. The one that passed is the bar1 precision pin, which was already true -- it documents scope, it does not test the fix. the headline: an abort injected AFTER the stop is still SEEN latch cleared on every registered transport, before the thread is gone, heterogeneous registry left alone, a raising transport does not strand the others, idempotent, semantics unchanged on a clean word stop: timed-out join keeps the handle, detach is logged, a second stop does not claim success, clean join clears and is quiet, idempotent, safe when never started, deadline derived from the poll cadence wiring: graceful stops and re-arms, exception path leaves it alone, ALWAYS-STOP with no gate, no watchdog is a quiet no-op, never raises ordering: release_distributed records stop_watchdog BEFORE destroy_model_parallel; source-order and AST call-site pins can-fail by breakage, FOUR independent neuters: re-arm removed -> 5 red (incl. the blindness test) handle cleared before the join -> 3 red ordering precondition removed -> 1 red (the by-construction test) scheduler call site removed -> 2 red regression, name-level with --color=no against a removed-diff baseline: distributed + managers + debug baseline 31 failed / 5199 passed with fix 29 failed / 5225 passed ZERO new failures. Two baseline failures did not reproduce (test_load_snapshot_backends ZmqRoundTrip, test_phase_flip_mover_streaming_631 staging price) -- both unrelated to barlink teardown, so they are FLAKY, not fixed by this change, and no credit is claimed for them. A harness bug of my own, found by running: the first watchdog stand-in used a worker that returned immediately, so every stop reported "already stopped" and four wiring tests were asserting nothing. The default worker now stays alive until stopped. FILING UPDATE: docs/FINDING_673_barlink_watchdog_stop_blocked.md carries the resolution paragraph and the scope correction. The sgl-project#673 family now stands at 5 of 6 abort-shaped items desk-addressed, 1 filed with no action (the vendored mooncake/mori/nixl workers -- no stop mechanism to wire, PD-only). ON-METAL PROOF REMAINS DEFERRED for the whole family: the abort is intermittent per process, so a single clean boot proves nothing. The fixes ride along on future teardown-armed boots.

…d conflict map PREPARATION ONLY. No branch pointer was moved and nothing was merged. Every conflict is MEASURED by trial merges in a throwaway worktree (throwaway/merge-train-probe, removed after measurement), not predicted. Built from git rather than from the handed list, and git contradicted the list twice: * fix/713-admission-intake is ALREADY in the serving line (+0 commits, 0 files) -- not a train item, and scheduling it would be a no-op step that looks like progress. * fix/728-max-bytes-uniform points at 79216e6, byte-identical to feat/706-phase-uniform-hicache-keys, and has NO REMOTE. It must not be merged as a separate item, and its identity has to be settled first: a local-only branch is the one kind that disappears with its worktree. DIVERGENCE. The 4222976 class turned out not to be the risk it looked like: that commit is PRESENT in integration/r2 and absent only from upstream main, which is true of every fork commit. The useful axis is the merge target -- 167 commits are on the serving line and not in integration/r2, and exactly SIX of them are on no other train branch (the sgl-project#677 range c4bc982..5fed8a6, sgl-project#708 fc6f97b, and the merge 761d0d7). All six are reachable from feat/677-park-wiring, so that branch carries them; if any step drops or rewrites it, those six are the loss. The serving line is also NOT an orphaned detached head -- 5fed8a6 is exactly feat/677-park-wiring, checked, because unbranched commits on a serving head are how a train loses a fix. ORDER. All eleven candidates are independent siblings (a containment check found no branch containing another), so order is a conflict question. The measured result is two clusters and a clean set: * Cluster A, scheduler_teardown.py: all four sgl-project#673 thread-stop branches add their stop logic to the same file this lane created for sgl-project#673. They conflict with EACH OTHER, not with the trunk, so whichever lands first sets the file's shape. The barlink one also touches scheduler.py and belongs to live sgl-project#722. * Cluster B, planner/seam: fix/602-fill-side, fix/701-ledger-wiring and feat/704-prefill-ladder each rewrote seam_slope.py, planner/pp_cut.py and test_pp_cut_prefill_speed_702.py. Three lanes editing one model of the same thing -- the conflict is SEMANTIC, and resolving it by taking hunks would produce a seam model nobody designed. * The clean six-step train (621, 699, 673-lockstep, 706, 717, 677) merged in sequence with no conflicts at all and can run without either cluster. TEST MATRIX, measured on the merged probe state: mem_cache 940 failed / 973 passed and distributed 21 failed / 2716 passed, both matching the standing baselines. managers came out at 12 failed / 2274 passed against a standing figure of 14 -- and the point is that this suite's count is TRAIN-COMPOSITION-DEPENDENT: this lane's branch alone shows 4, the merged probe 12, with the extra 8 arriving from the sgl-project#677/sgl-project#713/sgl-project#631 lanes rather than from the merge. The failing classes are listed so they can be attributed, and each owner must record their own baseline before the train, or "the branch shipped it" becomes "the merge broke it". HELD OUT, with the separability question answered: 2ce1ed7 is NOT docs-only -- it changes managers/phase_flip_runtime.py, which is exactly why it earns its review boot. The two later sgl-project#441 commits touch no file it touches, so they are cherry-pickable without it; the only entanglement on that branch is be1fcec -> 2ce1ed7 (same NOTE). But 4512136 carries sgl-kernel/csrc/kvcacheio/transfer.cu, so riding it means a kernel rebuild -- its own boot-gated risk, not something to smuggle in behind a docs-and-tests framing. Also in this push: the sgl-project#568 ledger commit (79216e6), which the audit found was this lane's only unpushed work.

…c, cross-ref sgl-project#726 Docs only; no production file touched. Closes the three items my own retro-sweep left open. ## 1. EVERY=32 (former section 19.3): NOT DEAD, but path-dependent Traced the constant to its consumers rather than trusting either ledger entry. check_every() returns 1 when the env var is unset (barlink_abort_gate.py: 189-197). The default is ONE, not 32 -- "=32" is only meaningful if somebody sets it. barlink_bar1.py:5177 reaches it UNCONDITIONALLY, after the abort_check_enabled / pending-launch gates. LIVE on bar1. barlink_device.py:1562 sits in the in-line fallback path BELOW sgl-project#517's _abort_poll_active early return, so on the device transport it is bypassed entirely whenever the watchdog feeds the flag. BOTH LEDGER READINGS WERE PARTIALLY RIGHT AND NEITHER WAS COMPLETE. "Dead since sgl-project#517" is true of the DEVICE path only -- that is exactly what phase 2 made cheap -- while bar1 never got that treatment and still consults the knob. NOTE_517 itself treated ..._EVERY as arm B3, expected to buy nothing on top of the staged read; "buys nothing measurable" is not "the consumer was removed", and that elision is how the two readings drifted apart. The sgl-project#431 utilisation figure (72-84% -> 86-91%) predates sgl-project#517 and was never re-measured after it. THE RECIPE QUESTION, answered: the production boot recipe does NOT set it -- absent from startkommandos-rig.md (grep -c = 0). So there is no dead knob in the recipe misleading the next operator, which was the specific worry. Worth recording: this is the SAME bar1-vs-device asymmetry I found independently in sgl-project#673, where bar1's check_aborted never consults _abort_poll_active while the device path short-circuits on it. Two investigations converged on one structural fact about these two transports. Section 19.3 is now empty and points at the settled 19.1 entry. ## 2. Stale spill-matrix doc (section 19.4): corrected at both sites ANALYSE_spill_matrix_20260804.md claimed kvso x HiCache is mutually exclusive and that "kvso cannot run on the production recipe at all". sgl-project#550 superseded that: the combination is OPT-IN, gated on KVSO_ALLOW_HICACHE (server_args.py:7502, `if os.environ.get("KVSO_ALLOW_HICACHE", "0") != "1": raise`). Per the refusal's own text the two host pools are independent objects with DISJOINT KEY SPACES and their pinned host RAM is summed by ONE JOINT BUDGET GUARD instead of each validating alone; what remains is a measurement (spill-copy vs prefetch contention), not a mechanism. Fixed in section S1 and in the H15 matrix row, with the original text kept inline so the change is legible rather than silently rewritten. ANCHOR CORRECTION: the gate is at server_args.py:7502, NOT the :7385-7395 given in the brief. Cited by symbol per the anchor rule -- the sgl-project#621 sweep found audit line numbers had drifted, two of three into the wrong file. ## 3. sgl-project#726 cross-reference added to section 19.1 The INT8-KV IMMA-QK builder must CREATE the dtype surface, not find it: choices at server_args.py:1026 carry no int8, the fp8 KV scale path is hard-coded per-tensor, and the one per-group-scale precedent (MHATokenToKVPoolFP4) dequantises eagerly before any backend sees it -- copying it buys VRAM savings but NOT the bandwidth savings an int8-KV lane exists for. Budget the plumbing as new work. The anchor is VERIFIED; the FP4-precedent detail is marked UNVERIFIED-LEDGER (relayed from ANALYSE_726, not re-read here). PRIOR-ART GATE honoured: grepped FEATURE_CATALOG, docs/dev and git log --all --grep before each change; both absence claims carry the file:line of the refusing gate (KVSO_ALLOW_HICACHE at :7502, kv_cache_dtype choices at :1026).

Revert "Temporary fix invalid sample results (#668)"

2517225

This reverts commit 8f4b155.

hnyls2002 merged commit 39c5731 into main Jul 20, 2024

hnyls2002 deleted the revert-668-fix-sample branch July 20, 2024 09:06

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

Revert "Temporary fix invalid sample results" (sgl-project#673)

f6baa53

amote-i pushed a commit to amote-i/sglang that referenced this pull request Dec 8, 2025

[feat] Use einsum matmul op in MLA matrix absorb (sgl-project#673)

ab4561b

* [feat] Use einsum matmul op in MLA matrix absorb

Navigation Menu

Uh oh!

Revert "Temporary fix invalid sample results" - #673

Revert "Temporary fix invalid sample results"#673
hnyls2002 merged 1 commit into
mainfrom
revert-668-fix-sample

hnyls2002 commented Jul 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hnyls2002 commented Jul 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant