Skip to content

Improve the coverage of the openai api server test - #878

Merged
Ying1123 merged 2 commits into
mainfrom
openai-server-test
Aug 1, 2024
Merged

Ying1123 merged 2 commits into
mainfrom
openai-server-test

Conversation

@Ying1123

@Ying1123 Ying1123 commented Aug 1, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@Ying1123
Ying1123 merged commit 60340a3 into main Aug 1, 2024
@Ying1123
Ying1123 deleted the openai-server-test branch August 1, 2024 23:01
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 26, 2026
…l-project#876, which the register owns

sgl-project#876 is the code-graph infrastructure (devindex/zoekt) in the register. I had
taken it in-tree for two unrelated findings, so `grep 'sgl-project#876'` would have landed a
future reader between two different subjects -- the anchor-drift class this
register has been hit by before.

  sgl-project#877  the implicit gather temporary in the in-place row move (was sgl-project#876)
  sgl-project#878  the reversed copy-back precondition (was #876b)

NINE anchor lines, not the three that were visible from the outside. Six were in
places a spot-check would not reach: the INPLACE_MOVE_MAX_ROWS declaration, the
runtime warning STRING (which would have printed a wrong ticket into a boot log,
the worst copy of all), the per-layer hoist comment, and both test module
docstrings.

BOTH TEST FILENAMES RENAMED TOO. A filename carrying a ticket number is a
greppable anchor like any other, and this tree's convention puts the number
there; leaving `test_inplace_move_bound_876.py` would have reproduced the exact
collision one directory over. Neither file is under the tier-2 gate's path
(`unit/managers`), so the partition table is untouched.

Ordered so the passes cannot collide: #876b -> sgl-project#878 FIRST, then sgl-project#876 -> sgl-project#877.
The reverse order would have rewritten `#876b` into `#877b`.

Commit messages on the pushed commits keep the old numbers -- rewriting them
needs a force-push, which needs the user's word. The code is what gets grepped
and the code is now right; the register carries the discrepancy.

Verified: no `sgl-project#876` anchor remains anywhere in python/ or test/. The residual
literal "876" matches are unrelated -- upstream PR numbers, float digits,
bootstrap ports.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 26, 2026
…second primitive

POSTEN 4, and checking the premise changed the answer. The question was posed as
"the marker must survive a process restart". It does not have to, because the
thing it describes does not.

THE IMAGE IS RE-PRIMED AT EVERY BOOT. `prime_arena_from_image`
(phase_flip_boot.py:499) calls the same `rotate_arena` with `outgoing_bytes=0`,
which degenerates the rotation to a plain contiguous H2D and fills the image
from the model in canonical forward order. Pinned host memory does not outlive
the process, and even the file-backed arm is overwritten by that fill before any
flip runs. So parity only has to hold from one flip to the next within one
process lifetime.

A LEGAL HOME THEREFORE ALREADY EXISTS: `PhaseFlipStacks.image_holds`, the
in-process marker recording WHICH layout the image contains. Recording WHICH
ORDER is the same kind of fact with the same lifetime, in the same place. No
sidecar, no manifest, no second trailer slot -- the refusal I was prepared to
write is not needed.

THAT SOLVES STORAGE AND NOT VERIFICATION, and I had conflated them. A wrong
`image_holds` is caught today: `rotate_arena` raises RotationHazard on
`image_holds != wants`, and the device-side checksum catches the rest. A wrong
PARITY is caught by nothing, because the checksum is an order-blind sum. So the
scheme still needs a second verification primitive -- it just does not need a
second storage slot. Those were two questions and only one of them was open.

THE ORDER INFORMATION IS ALREADY COMPUTED AND THEN DISCARDED. `uint8_checksum`
builds a per-chunk sum VECTOR and collapses it with a final `.sum()`
(weights_arena.py:125-129). The vector is order-sensitive; the collapse is what
throws it away. Retaining it costs no extra data movement.

IT IS NOT FREE, AND THIS CONSTRAINT IS INVISIBLE WITHOUT READING WHY. The chunk
size is ADAPTIVE -- `_checksum_chunk_bytes` sizes it from free device memory --
and `uint8_checksum`'s docstring makes chunk-size-independence load-bearing so
"two ranks with different free memory still agree". A per-chunk vector under
adaptive chunking is not comparable across ranks or across a flip. The
order-sensitive primitive must use a FIXED partition, deliberately decoupled
from the free-memory heuristic.

THE SCHEME IS STILL NOT BUILT, per the standing instruction: the marker's
verification is what was missing, and it still is.

CAN-FAIL: three mutants. Making the chunk sizing non-adaptive reds the
constraint test; removing the per-chunk split reds the "already computed" test;
giving the priming fill a copy-back reds the canonical-forward test.

THAT THIRD MUTANT FIRST SURVIVED, and the reason is worth recording. The test
asserted `"outgoing_bytes=0" in inspect.getsource(...)` -- and that string also
appears in the function's own DOCSTRING, so the assertion passed on the prose
while the real argument changed. Same shape as this branch's 4 KiB-versus-1 MiB
miss. Now asserted through the AST on the actual `rotate_arena(...)` keyword,
and the mutant is caught.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 26, 2026
… my 18 commits: two claims withdrawn

The sweep I named and had not run. Every citation of a log string or a borrowed
measurement across all 18 commits on this branch, checked against THE source it
cites -- not against any source. 31 citations, four states.

BELEGT -- 27. Everything drawn from
/spinning/evidence-665-f1/boot_w40_857strict_0826_0516.log verifies literally in
that file: the three segment times, the 75 % worst-share, `save 4.342`,
`gpu-span d2h 0.000s / h2d 0.000s` (21x), `checksum 0.319`,
`gdn_state->refill_highwater` 0.2 and 7.0, `weights_refill->cutover` 113.5 and
972.1, the 498/268 aliasing counts, `REFILL pp_to_tp took 3.961 s for 8573.8
MiB`, `pp_attn_stage_ratio=[8, 4, 4]`, `disaggregation_mode='null'`,
`rank_tp_ratio=None`, `uneven_token_vector='29,19,16'`, and the W40 specimen
details I repeated from the briefing (05:20:50, the rid, the IndexError,
`SEAM RESTORE ATTEMPT`, `FLIP EXTENT PROBE`, `extent=13`,
`transfer_layer_num=32`). The log is present; source not missing.

NIE-GEDRUCKT, CLAIM ALSO FELL -- 1. "the boot line at :3228 prints
`get_total_num_kv_heads()`": zero occurrences. Already retracted in 19b8177;
the sentence it supported was false and the true one is restored.

NIE-GEDRUCKT, CLAIM SURVIVES -- 1. "24 s after health 200". The string
`health 200` never printed. The log says `"GET /health HTTP/1.1" 200 OK` at
05:20:26, and the crash is at 05:20:50: exactly 24 s. The claim is right to the
second; only my citation form was ungreppable, so anyone verifying it would have
found nothing. The two cases are kept apart because their consequences differ.

MISATTRIBUTED, AND MISREAD -- 1. I priced sgl-project#875's DO-NOT-BUILD verdict with
"43.9 KiB per crossing, 166 us enqueue, 1777-9201 us receive", attributed to
sgl-project#656. It is not in sgl-project#656. It is in 13c55d7 "[PP] sgl-project#201 slice 2: the stage
boundary across two rigs" -- CROSS-RIG, two nodes, a 40G line, a different model
(Qwen3.5-4B fp16), timing PP microbatch crossings and not an intra-node
collective. And its own text refuses my reading: "`recv` is BLOCKING, i.e. bubble
plus wire -- 9.2 ms on stage 1 is that stage waiting for stage 0, NOT the 40G
line". I used a pipeline bubble as transport latency, from the wrong ticket, for
the wrong link. WITHDRAWN, with no substitute: I have no measured local
collective figure.

QUELLE-FEHLT -- 1. "sgl-project#721's measured cgroup peak (111.3 of 118 GiB,
oom_kill=17)", used in sgl-project#877 and in sgl-project#878's headroom. `oom_kill=17` has NO in-tree
source, and the only `111.3` is `111.3%` in a benchmark table
(docs/dev/631/PROD_BRINGUP_BENCH.md) -- an unrelated percentage. It came from a
briefing and I repeated it as measured, twice. WITHDRAWN. This is the shape the
operator hit this morning with the retracted "1833/1833 records".

WHAT THE TWO WITHDRAWALS COST, stated rather than smoothed:
  * sgl-project#875's verdict loses its timing leg entirely. It now rests on two terms that
    need no transport number: the HEAD axis is LOSSY (PP 4 kv-heads per layer,
    TP 1), so PP->TP is not a remap until someone decides which heads survive
    and no such rule exists; and a new collective in the no-return region is the
    sgl-project#630 wedge shape, an unbounded risk. Point one alone is decisive. DO NOT
    BUILD stands, on better ground than it had.
  * sgl-project#878's headroom argument is re-grounded on a figure I took myself: this box
    reports 128711 MB total / 76971 MB free, so +96 MiB of pinned host image is
    negligible by four orders of magnitude, needing nobody's peak.
  * sgl-project#877's conclusion is untouched: N <= 72 rows of a per-LAYER temporary is
    kilobytes, and separately none of its six sites is constructed on this rig.

THE CLASS: a citation is evidence only for the claim it is attached to, in the
source it names. Three of the four defects here were true-sounding numbers from
the right neighbourhood -- a real measurement of a different link, a real
percentage read as gigabytes, a real code branch that never executes.
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.

1 participant