Broadcast documents_seen as the model version in weights_ready#561
Merged
Conversation
This was referenced Jul 8, 2026
The `weights_ready` event now carries `document_count` (= documents_seen) alongside the existing `step` (= completed step). Consumers stamp `document_count` onto rollouts so staleness is measured in documents, aligning with DeepSpeed's document clock, while `step` remains available for logging. Keeping `step` makes the change backward-compatible. Threads `documents_seen` through the `TrainerCallback.run_begin` / `step_end` hooks (a training-progress counter alongside `step`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…it in test - Rename the wire field document_count -> documents_seen for consistency with the internal counter and the rest of the codebase (no shipped consumer yet). - Reword the broadcast comment to describe the fields locally, without naming downstream consumers. - Assert the new documents_seen field is present and non-negative in the streaming consumer test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jlamypoirier
force-pushed
the
jlp_weights_ready_version
branch
from
d0d04aa to
bb91627
Compare
…sion # Conflicts: # fast_llm/engine/training/config.py # fast_llm/engine/training/streaming.py # fast_llm/engine/training/trainer.py
… restating comment Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude Opus 4.8, on behalf of @jlamypoirier.
Summary
Extracted from #553. Split from the reward / per-token-
model_versionpiece (briefly combined in#557, now closed) since the two have no code dependency — this is purely the producer-side
broadcast; the other PR is a data-pipeline consumer of a different field.
weights_readyevent now carriesdocuments_seen(the cumulative document count, whichdoubles as the model version) alongside the existing
step(= completed step). A consumer canstamp
documents_seenonto rollouts so staleness is measured in documents, aligning with thedocument clock;
stepremains available for logging. Keepingstepmakes thisbackward-compatible.
documents_seenthrough theTrainerCallback.run_begin/step_endhooks.The counter itself (
Trainer._documents_seen) landed in #559 and is now onmain; this branch ismerged up to current
main(which also carries the weight-sync-time metric from #560, with whichstep_endintegrates cleanly). The paired PipelineRL change (readingdocuments_seenas the modelversion) is a separate PR against PipelineRL's
fast-llmbranch.Tests
tests/data/test_streaming.py: passes (CPU).tests/models/test_streaming.py(GPU end-to-end) now asserts eachweights_readymessage carriesa non-decreasing
documents_seen; not run here (no GPU) — worth running in CI before merge.