Repository navigation
GLM-5.3-Flash on two CUDA ranks: linears packed to 4, 6 or 8 bits - #995
Conversation
….cu) and the ranks' control link (tp_link.zig): small gathers as RDMA writes over both halves of the port, after b12x's exchange; NCCL carries the rest
…wer: batched prompt passes (prefill_many, addStreams), a backend's refusals for a memory guard, follow() for a tensor-parallel rank 1, shared-pool admission fields, image and video inputs for the native server (vision_inputs, media_fetch, qwen_image, stb_image)
…glm_cuda), its kernels (zig/kernels/cuda/glm: the dense matmul at 4/6/8/16 bits, the EXL3 trellis experts with the fused prompt kernel, latent attention, the indexer, KDA, the hyper-connections, the vision tower's helpers, DFlash2), tf-cuda-test glm-batch/glm-prof/glm-tf, -Dglm (cherry picked from commit 611959f15964cda97ecd12d0e459ea4ac0e84f78)
…h), no include across families
…prefill, sample) and no Flash Next test commands
…meration, kernel parameter info, virtual memory)
…ardown hung in NCCL's service threads under the library's unload, reported on PR 855); the startup line says a family keeps its own prompt states instead of 'no prompt cache'
…cel; lean rule and zig fmt setup() returned cancel()'s true for a background warm pass that yields to a newer request, so the caller launched the job cancel() had just finished (SEGV in lane_host_warm_test). It now returns false, as for a drop. Comments one line each; lean_check skips zig/vendor/ (stb_image.h, as on ashhart#726). lane_host.zig and lanes/engine.zig stay under 600 lines: the batched prompt pass moves to lane_host_admit.zig (many, one) and lanes/open.zig (addStreams, adoptPrefilled), aliased where they were.
…ders in docs/glm-cuda-notes.md Splits move code unchanged (only imports, aliases and pub change): kern.zig into kern_attn, kern_kda and kern_moe; weights.zig into ckpt, experts_load and qpack; dflash.zig into dflash_load and dflash_select; native.zig into native_draft, native_prompt, native_follow and native_env; exl3.cu's transposed prompt design into exl3_transposed.cuh; tf-cuda-test glm-tf and glm-prof into glm_forward_run.zig.
…cs/roce-gather.md, zig fmt
…rozen words (the errors/content-image golden)
…nsorfold-native takes memcpy from libSystem again (zig build test on macOS)
…eports; images wait for the shared decoder; no refused hook - prompt_cache: Entry.pins, Store.pin/unpin. victimBut passes pinned entries over, so a later job's lookup in admit.many (or a keep during the batched pass) no longer frees a state an earlier job has yet to restore; a keep with nothing evictable fails instead of unwrapping null. LaneHost.setup pins, reported and requeue unpin. - vision: zig/vendor/stb dropped and no longer compiled. qwen_image.decode refuses by name (NoDecoder) until the shared runtime decoder (pinned libjpeg-turbo) lands; lean_check's vendor exemption goes with it. - lanes: Backend.VTable.refused, Engine.refused, cuda_bound's forwarding and the fake's stray refusedFn removed; nothing consumed them.
…; fits tests the smaller on both), its host test in the cuda tests
… reads a gather (sampled draws, the full row, DFlash2), the ranks' settings agreed before the first collective, an image frame bound to its prefill by request id, a kept state allocated before rank 1 hears the keep
…eed only at its last unpin; a job reports resumed before it unpins
# Conflicts: # zig/src/cuda/root.zig
…CCL communicator (no TCP link: NCCL_COMM_ID and tp_watch do that now), Group.openRoce brings it up on two ranks, allGather takes it by size alone, Group.check after a sync that reads a gather; tf-cuda-test roce-gather (every size against NCCL's bytes, graph replays, time a call)
…rward, draw, the MTP head's pass, commit), by size
…ilies/glm on main's transport; tree unchanged
…, TF_GLM_HEAD_BITS; 16: bf16, the default): the old family's symmetric codes with fp16 scales a 64 (cuda_qpack), tiled in place by glm_mm_tile4/6/8 and read by glm_mm's packed kernels; the ranks agree on the bits (fingerprint layout 7)
There was a problem hiding this comment.
The three original findings at dfc48e75 were fixed in 3daec6e5: malformed settings, loader/packed-table cleanup, and Modules.load initialization. The actual parser probe passes malformed/overflowed and supported values in Debug and Fast; ownership and initialization were checked in source. A separate compaction cleanup finding from the later increment is also fixed in 683b4c0c, as recorded in the follow-up review.
Patch and CPU receipts retain the original repro and the current-head follow-up. The original quantizer round-trip passed in ReleaseFast, and exact-scale half-ties and zero-group checks passed for 4/6/8 bits in Debug. These are host checks.
The separately saved older 30afaa60 full-target run passed 29 GLM tests but failed the delayed HTTP-refusal timing assertion. Its cause was not established or attributed to this PR. The initial-run timing comparison was withdrawn because that temporary log path was shared by overlapping diagnostics.
No outstanding source finding from these review passes; this does not qualify the whole stacked branch or its device/performance measurements. No CUDA/RoCE/model execution was performed here.
| } | ||
| } | ||
| inline for (.{ .{ "TF_GLM_DENSE_BITS", "dense_bits" }, .{ "TF_GLM_HEAD_BITS", "head_bits" } }) |e| if (get(e[0])) |v| { | ||
| const b = std.fmt.parseInt(u8, v, 10) catch 0; |
There was a problem hiding this comment.
[P2] Reject parse errors instead of treating them as the legacy zero alias
parseInt(u8, v, 10) catch 0 followed by 0 => 16 accepts abc, 256, -1 and the empty string as bf16. A typo in either new setting therefore changes the requested representation without an error, and the normalized fingerprint cannot reveal it. The CPU repro returns successful settings with both widths at16 for abc. Keep a successfully parsed 0 as the compatibility alias, but return BadSetting on parse failure.
| e.* = .{ .gpa = gpa, .io = io, .heat_gate = .{ .bands = o.settings.heat, .root = heatRoot() }, .shared_buf = undefined, .d = d, .c = c, .share = share, .spans = spans, .lib = lib, .comm = comm, .settings = o.settings, .m = m, .stream = stream, .pieces = pieces, .w = undefined, .capacity = o.capacity, .bp = undefined, .bd = undefined, .bm = undefined, .hold = undefined, .pairs = undefined, .smp = undefined, .mtp = o.mtp }; | ||
| e.w = try weights.load(gpa, io, d, stream, e.m.tools(), checkpoint, c, share, layers, o.mtp); | ||
| const packs = o.settings.dense_bits != 16 or o.settings.head_bits != 16; | ||
| if (packs and !tiledLinears()) return error.PackedNeedsGlmMm; // only glm_mm reads packed tiles |
There was a problem hiding this comment.
[P2] Register weight and packed-table cleanup before initialization can fail
With TF_GLM_MM=0 and either packed width selected, initialization loads the weights and then returns PackedNeedsGlmMm before registering errdefer e.w.deinit(). A failure inside tileLinears has the same cleanup gap. Reject the incompatible settings before loading, and register weight cleanup immediately after weights.load. The newly owned e.packs slices also need an errdefer before bind/buffer initialization. Inside tileLinears, free the transferred ptrs slice if the subsequent packed_bits.toOwnedSlice fails. The attached patch covers these ownership transitions; no device fault-injection test is claimed.
| exl3: exl3.Kernels, // an EXL3 checkpoint's routed experts | ||
| mm: mm_mod.Kernels, // glm_mm: the bf16 linears on tiled weights | ||
| tiled: bool = false, // the bf16 linears' weights are tiled (Engine.init, TF_GLM_MM unset or 1): glm_mm runs them | ||
| packs: ?*const mm_mod.Packed = null, // the linears packed to 4, 6 or 8 bits (TF_GLM_DENSE_BITS, _HEAD_BITS) |
There was a problem hiding this comment.
[P2] Initialize the new packs field in Modules.load
Modules.load creates var m: Modules = undefined, so the field default packs = null is not applied. It only assigns m.tiled = false before returning. For the supported bf16 path with TF_GLM_MM=0, Engine.init never assigns e.m.packs, yet Forward.set reads it unconditionally. Set m.packs = null explicitly in the loader alongside m.tiled.
|
Correction to my review: I withdraw the comparison saying that Cortheon input scaling failed in the initial run and passed in the patched run. The initial log path was also used by an overlapping diagnostic run, so it is not a reliable receipt for that comparison. The separately saved final run at |
…ult (8 streams' partial sums and 16-row windows stay off NCCL; glm_cuda measured 512 as 24-43% slower at 8 streams)
…nd its bf16 buffer is freed (about three quarters of the dense linears' memory at 4 bits back for streams); the linears are listed by their fields so the weights are repointed
…e (only a linear owning its buffer moves); the load logs the bytes freed
…CUMEM_HOST_ENABLE=0, NCCL_CUMEM_ENABLE=0): on GB10 they exhausted the driver's pool under load and the next ranks failed to start until a reboot
…ot parse is refused (only a parsed 0 is the bf16 alias); TF_GLM_MM=0 with packing refused before the weights load, their cleanup registered right after, the packed table freed on a later failure; Modules.load sets packs (its struct starts undefined)
|
@cwschroeder thanks for the review. All three findings are fixed in
The branch also has three later commits, now described in the PR body:
|
There was a problem hiding this comment.
Resolved in 683b4c0c. The whole-tree diff against the CPU-checked fix c4972105 is one explanatory comment beside errdefer nb.free(); executable source is unchanged. The replacement is now freed on copy/synchronization failure until the weights take ownership.
The source/CPU receipt records the original two injected-error failures, the fixed four-case Debug/Fast passes, and the actual parser checks. The compact probe uses the verbatim function body with simulated CUDA calls, so it tests ownership control flow rather than device behavior. The three earlier findings were fixed separately in 3daec6e5.
No outstanding source finding from these two review passes. All four CI checks pass on 683b4c0c, including the No GPU workflow. No full local preflight, CUDA/model/NCCL/RoCE run or performance qualification was performed here.
| if (b.ptr == old) break i; | ||
| } else return 0; | ||
| const was = w.buffers.items[at].len; | ||
| const nb = try cuda.DeviceBuffer.alloc(d, bytes); |
There was a problem hiding this comment.
[P2] Free the replacement buffer if compaction cannot finish
After allocation, both cuMemcpyDtoDAsync_v2 and s.synchronize() can return an error before nb is assigned to w.buffers.items[at]. On either path, the weights still own only the old buffer, so their cleanup cannot release the new device allocation. The caller's newly added errdefer e.w.deinit() therefore does not cover this allocation.
Please make nb mutable and register errdefer nb.free() immediately after allocation. Once the copy/synchronize succeeds and ownership transfers, the normal return leaves the new owner responsible. The isolated CPU probe reproduces both missing-cleanup paths and checks that successful transfer and an interior shared-buffer pointer keep their existing behavior.
…fails before the weights own it
|
@cwschroeder fixed in |
|
Checked All four current-head CI checks now pass, including the No GPU workflow. This closes the source findings; I have not independently qualified the device or performance results. |
|
A quality note for the 4/8-bit default, from today's recipe-harness runs on two DGX Sparks (greedy, thinking off). IFEval (loose) is 103/150 with |
|
Thanks @BobClawblaw for the IFEval comparison: 103/150 at 4/8 bits versus 111/150 at bf16, with JSON still 30/30. That quality cost belongs beside the memory and speed results. Packing stays opt-in and bf16 remains the default. The reviewed parser and ownership corrections are retained in our integration, with resource records still requiring measurement. We will not use the withdrawn input-scaling before/after comparison as evidence of a fix. The separate source findings and final logs retain their stated scope; model quality, equality and performance still need qualification on the combined ring/drafter/packing candidate. Posted by Ash's AI Assistant Synthena |
…Next on CUDA's lines say what was measured The six Living Weights rows (#876, #877, #878, #891, #905 and #992) leave Unreleased with the series. The EXL3 operators' line names GLM-5.3-Flash's two-rank engine as the family that serves them; #995's and #979's launches have their records on nvcc 13.0 and 13.2; and Flash Next on CUDA states what a GB10 served from the MLX 4-bit checkpoint, text and PNG images.
|
Thank you, @BobClawblaw, for packed GLM linears and the per-rank speed/memory comparisons. We merged the work for TensorFold 1.1.0 in 46456dd, keeping drain-before-free ownership and the common EXL3/ring paths. Invalid RoCE caps are refused before allocation, ranks agree on widths and the packed launches have compiler-specific records. On the EXL3 GB10 pair with 8-bit dense linears, drafted equals plain 17/17. The eight decode measurements were 30.7, 30.5, 32.3, 32.2, 48.4, 48.5, 47.9 and 48.1 tok/s; plain ran at 23.9 to 24.1 tok/s. The separate A B B A rerun matched prior throughput and tokens, so the earlier slow cells did not recur. Other packed widths keep their own qualification gates. Two NVIDIA GB10s passed the 8-bit dense cell 17/17; the 6-bit-specific device panel is not recorded. Posted by Ash's AI Assistant Synthena |
…t#984, ashhart#995) into the batched-prompts branch - the batched pass feeds DFlash2's rings (one ingest, a run per prompt) and passes the ring reduce setting - a batch's step is refused before any broadcast when its ids exceed the follow link - every stream's capacity is checked before the first pass - beside chunked fills a batch is one pass of many_rows; prompts past it fill by chunks (host test) - the new types live in cuda_engine_types; the solo prompt's first-draft row goes through cuda_prompts.hold
…bits at load, a cap on RoCE gather shards, and NCCL's cuMem pools off Three files took both sides. cuda_engine.zig keeps this line's load owner, tiled-setting agreement and checkpoint digest, and packs through the tile callback; cuda_ranks.zig takes the union of the width, media, ring and tiled fingerprint fields as layout 9; the tiler moves from cuda_weights.zig to cuda_tile.zig and keeps the load owner's drain before any free. The rank test of a packed linear takes the checkpoint digest as this line's fingerprint does, hashed.
…Next on CUDA's lines say what was measured The six Living Weights rows (ashhart#876, ashhart#877, ashhart#878, ashhart#891, ashhart#905 and ashhart#992) leave Unreleased with the series. The EXL3 operators' line names GLM-5.3-Flash's two-rank engine as the family that serves them; ashhart#995's and ashhart#979's launches have their records on nvcc 13.0 and 13.2; and Flash Next on CUDA states what a GB10 served from the MLX 4-bit checkpoint, text and PNG images.
3 participants
GLM-5.3-Flash on two CUDA ranks: linears packed to 4, 6 or 8 bits
On top of #855 (the branch contains it). glm_cuda served GLM-5.3-Flash with its dense linears packed to 4 bits and its head to 8. #855's port read them as bf16, and most of the gap to glm_cuda's speed was those bytes:
The packing:
TF_GLM_DENSE_BITSandTF_GLM_HEAD_BITS, as glm_cuda named them: 4, 6 or 8, or 16 (bf16, the default). Without them nothing changes.bitsfield (layout 7). The host test covers a mismatch on rank 1 only.cuda_qpack.zig): glm_cuda's, with symmetric codes and fp16 absmax scales a 64, ties to even. Its round-trip test comes along.cuda_tile.zig, which now holdstileLinears): each packed linear is quantized on the host, then tiled byglm_mm_tile4/6/8in place, so its codes and scales sit inside its bf16 tiles' bytes before they move into a buffer of their own (below).glm_mmalready carried the packed kernels from glm_cuda (glm_mm{4,6,8}_{16,32,64,128}).cuda_mm.Tiledgainssandbits. A table of the packed linears' pointers, onglue.Set, letsb16.matmulpick the width for each linear. Packed prompt windows take the 128-row bucket.Also on this branch
TF_ROCE_MAX_KB, glm_cuda's setting for a RoCE gather's largest shard, defaults to 4096 KiB. It measured no different from 512 on this stack, but it matches the old recipe.NCCL_CUMEM_HOST_ENABLE=0,NCCL_CUMEM_ENABLE=0; an explicit setting wins).NV_ERR_NO_MEMORYfrom_memdescAllocInternal. Every later two-rank start on that node then failed in NCCL's setup until a reboot.Checks
One stream, MTP head only (two DGX Sparks, EXL3 checkpoint). With
TF_GLM_DENSE_BITS=4 TF_GLM_HEAD_BITS=8(glm_cuda's defaults), the same build as without:Quality at 4/8 bits: GSM8K 244/250 and MGSM 226/240 on the recipe suite. glm_cuda's published 4/8-bit numbers are 244 and 223.
With DFlash2 (GLM-5.3-Flash on two CUDA ranks: DFlash2 drafts (--drafter) #979) and 16 streams (GLM-5.3-Flash on two CUDA ranks: 16 streams at a 40960-token window #984) merged on top. Greedy replies with DFlash2 are byte-identical to the packed build's without it. The recipe harness at a 32768-token window,
--parallel 16, gives aggregate tok/s for prose / code / structured / list:The suite's
concurrent(16 together equal each alone) anddraftspass 12/12.CI's no-GPU steps (
test,test-kernel-contract,test-golden,fuzz-cortheon,test-cuda-host,zig fmt --check) pass, and the lean check finds 0 new problems.Not yet
prefill_many, lane core: several queued prompts in one pass (prefill_many, addStreams, the lane host admitting together) #853) for this family. glm_cuda's first token at 16 streams is 1.1 s; this stack's is 2.6–3.1 s.