Repository navigation
feat(weights): S3-compatible object store for the sparse weight broadcast - #4103
faresobeid wants to merge 14 commits into
Conversation
2fc1666 to
53ac64a
Compare
e7c9109 to
df525f5
Compare
cf00c16 to
4d27d57
Compare
4d27d57 to
89c3f38
Compare
42147b8 to
b74307b
Compare
4516802 to
8a16605
Compare
8a16605 to
8c220ce
Compare
9a5ace2 to
01b0801
Compare
8c75df3 to
c1036c9
Compare
c1036c9 to
49aa145
Compare
49aa145 to
204c7aa
Compare
cfc87c0 to
f302680
Compare
be8d24f to
4bc9e2d
Compare
f302680 to
ba56557
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ba56557. Configure here.
…g, keep a running engine layout on trainer resume, upload anchors before dropping their page cache
…hort preads to the end
ba56557 to
6052810
Compare
…se-weights-object-store
…se-weights-object-store
… broadcast, now that engines report at every full broadcast
| """With ``sparse``, publish a full checkpoint every this many steps. Engines that join or fall | ||
| off the delta chain load the last one and replay the deltas since.""" | ||
|
|
||
| storage: str | None = None |
There was a problem hiding this comment.
can we put this into a DeltaWeighBroadcastConfig and let fs bc config inherit from it.
then DeltaWeighBroadcastConfig is not None would be signal for doing sparse updates +
DeltaWeighBroadcastConfig.path = ... points to local/s3 bucket
There was a problem hiding this comment.
ah just seeing this is not against main. i think if we have DeltaWeightBroadcastConfig we dont need sparse anymore. and DeltaWeightBroadcastConfig.anchor_interval can just be DeltaWeightBroadcastConfig.interval and we explain that this specifies the interval we diff weights against
…se-weights-object-store # Conflicts: # src/prime_rl/inference/vllm/worker/filesystem.py # src/prime_rl/transports/weights/sparse.py
The S3/separate-dir storage is a field of the sparse config (no `sparse` check needed); the shared receiver reads it only for `sparse_filesystem`.
…se-weights-object-store
…age (local dir or s3://) The storage location of deltas and anchors is `DeltaWeightBroadcastConfig.path`; the receiver reads it from the filesystem config's `delta`.
…se-weights-object-store # Conflicts: # src/prime_rl/transports/weights/sparse.py
…se-weights-object-store # Conflicts: # src/prime_rl/transports/weights/filesystem.py
…se-weights-object-store
…se-weights-object-store

Stacked on #4085.
Adds
[weight_broadcast.delta] pathfor the sparse filesystem broadcast. It is unset by default (the broadcast dir, as today), or another dir, ors3://bucket/prefix, so that trainer and engines can run in different regions.utils/weight_store.py:S3Pathimplements the fewPathmethods the transport uses (/,name,parent,exists,glob,read_text).read_range/removework on bothPathandS3Path.sparse_weights.write_filegains an S3 branch: there is no rename on S3, so a file is one PUT, or a multipart upload in 64 MiB parts for anchors. Endpoint and credentials come from the standard AWS environment variables. boto3 is imported only when S3 is used.sparse_weights.py: the manifest, delta part files,rank_{r}.partsmarkers, anchors and engine layouts are all written withwrite_file, so a part or marker is visible only once complete. The file format is unchanged. Engines poll the store for part files as the trainer uploads them (on S3 too), and read only the frames they need with ranged GETs (preadon a filesystem). Anchors are read from the safetensors headers plus ranged reads, up to 32 tensors ahead.python -m prime_rl.utils.sparse_weights; vLLM workers are daemonic, so no multiprocessing), and the decoded frames come back through a local temporary file. Inside the worker, boto3 releases and retakes the GIL many times per request, and that crawls while the engine serves: 230-340 s per 30B delta, vs a few seconds from the child process.s3://.../step_Nto the engines, so inference needs no config. With a separatedelta.path(a bucket, the bucket roots3://bucket, or another dir), the trainer deletes old steps there and clears leftoverstep_*entries at startup, as it does for the broadcast dir. The master logs the delta size from the bytes each rank reports in its parts marker.Qwen3-30B-A3B bf16, 1 node (trainer 6 GPUs, engine TP2), 20 steps,
interval = 10, same commit for both columns. MinIO on node-local disk vs the shared filesystem. Medians over v2-v20, measured on an earlier head:All 20 deltas were checksum-verified on both engine workers in both runs.
Offline, on the same node, an anchor load takes 16.5 s from the filesystem and 63 s from MinIO. Fresh engine (measured before part streaming): replay from the v10 anchor to v15, then steps v16-v20: filesystem 41.7 s / 1.4-1.5 s per step; MinIO 102 s / 3.1-3.3 s per step. A delta applied twice fails the checksum check.
#4085 takes the deltas from the optimizer step. With a separate
delta.path, the trainer writes the delta parts straight into the storage step dir as the step produces them (there is no rename on S3). Anchors are written as local files next to the broadcast dir (their slices arrive in any order) and uploaded once complete, then theanchor_rank{r}.donemarker. Full checkpoints (a resumed run's first broadcast, SFT online evals) go through the same writer. At startup the trainer also clears old engine layouts under a reused prefix: workers of an earlier run would otherwise leave a stale fp8 layout for the trainer to read.Latest check (Qwen3-30B-A3B AdamW
offload = "optimizer", 10 steps, anchor every 5 steps, MinIO on node-local disk): 20/20 deltas verified on both engine workers, both anchors complete in the bucket; fwd/bwd 9.4 s on normal steps (7.0 s with the broadcast dir) and 12.6 / 18.8 s on anchor steps; orchestrator update 11.3 s. CPU round trips against MinIO pass: delta, subprocess read, an anchor written locally, uploaded and read back by flat range across rank slices, bucket-root paths, engine layout cleanup.All engines read the same bucket. A cache per region would be a replicated copy of the prefix (for example with bucket replication) plus a storage URL per engine; this PR does not implement it.
Validation
main), no conflicts. CPU on the merged head:pytest tests/unit/orchestrator tests/unit/test_configs.py tests/unit/utils tests/unit/transports tests/unit/inference -m "not gpu"502 passed;ruff check,ruff format --check,uv lock --checkclean.Before merge
main.Note
High Risk
Changes the critical trainer→engine weight sync path with new S3 I/O, subprocess delta reading, and split handshake vs data locations; misconfiguration or partial uploads could break updates despite checksum checks.
Overview
Adds
[weight_broadcast.delta] pathso sparse deltas and anchors can live on a separate directory ors3://bucket/prefix, while the broadcast handshake (finished markers, orchestration) stays on the trainer’s local broadcast dir. Engines receives3://.../step_Nfrom the orchestrator without extra inference config.Introduces
utils/weight_store.py(S3Path, rangedread_range, multipartupload,remove) and threadsStoragePaththroughsparse_weights, the sparse trainer sender, and the vLLM filesystem worker. Delta/anchor I/O uses ranged reads instead of localpreadwhere needed;write_fileuploads atomically to S3. On S3, the trainer streams delta parts straight into the remote step dir (no rename/move from hidden staging dirs); anchors are built locally next to the broadcast dir, then uploaded.vLLM workers call
read_delta_in_subprocessfor S3 so boto3’s GIL churn does not block serving during large delta downloads. Startup clears stalestep_*under a reused prefix; retention also deletes old steps on the separate storage path.docs/scaling.mddocuments cross-region setup (AWS env vars, boto3, orchestrator colocated with trainer).Reviewed by Cursor Bugbot for commit ce19c66. Bugbot is set up for automated code reviews on this repo. Configure here.