fix(stamp): take the scheduler horizon from the job's num_rounds (#503) - #520
Open
Ultimate-Storm wants to merge 1 commit into
Open
fix(stamp): take the scheduler horizon from the job's num_rounds (#503)#520Ultimate-Storm wants to merge 1 commit into
Ultimate-Storm wants to merge 1 commit into
Conversation
STAMP sizes its OneCycleLR scheduler once, for max_epochs x total_rounds steps.
The rounds actually executed come from num_rounds in the job's
config_fed_server.conf. These were two independent sources of truth: the client
read STAMP_NUM_ROUNDS from its environment and nothing checked they agreed. When
the server outran the scheduler, training died mid-run with
ValueError: Tried to step 9 times. The specified number of total steps is 8
surfaced to the server as EXECUTION_EXCEPTION -> FATAL_SYSTEM_ERROR. The message
names neither value. Worse, oversizing is harmless, so a mismatch stays invisible
until someone RAISES num_rounds — it then bites on a long run, hours in. This
aborted the first 1.7.0 validation at round 4 of 20.
The job's app, including config/, is deployed to every client, so the client can
read the authoritative value off disk rather than being told separately. Adds
resolve_total_rounds(), used for swarm runs:
job's num_rounds > STAMP_NUM_ROUNDS > default 20
When both are present and disagree the job wins, and the discrepancy is logged
loudly — following the environment instead is exactly what kills the run. When
the config cannot be read we fall back to the environment, and say so, rather
than guessing silently.
12 unit tests, including the exact 1.7.0 case (env said 2, the job ran 20).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ultimate-Storm
force-pushed
the
fix/503-total-rounds-single-source
branch
from
September 2, 2026 09:15
6809cb0 to
05b9d4a
Compare
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
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.
Closes #503.
The problem
STAMP sizes its OneCycleLR scheduler once, for
max_epochs x total_roundssteps. The rounds actually executed come fromnum_roundsin the job'sconfig_fed_server.conf. These were two independent sources of truth — the client readSTAMP_NUM_ROUNDSfrom its environment, and nothing checked they agreed.When the server outran the scheduler, training died mid-run with:
The message names neither value. Worse: oversizing is harmless, so a mismatch stays invisible until someone raises
num_rounds— it then bites hours into a long run. This aborted the first 1.7.0 validation at round 4 of 20.The fix
The job's app — including
config/— is deployed to every client, so the client can read the authoritative value off disk rather than being told separately. Verified on a live client:resolve_total_rounds()therefore resolves, for swarm runs:When both are present and disagree the job wins, and the discrepancy is logged loudly — following the environment is exactly what kills the run. When the config can't be read we fall back to the environment and say so, rather than guessing silently.
No fork patch was needed. (
FLModelhas atotal_roundsfield andFLModelUtilsmapsAppConstants.NUM_ROUNDSonto it, but CCWF never sets that header — confirmed both in the source and empirically in real run logs — so the client API route is not available today.)Testing
12 new unit tests covering precedence, the exact 1.7.0 case (env said 2, job ran 20), non-integer env, missing config, and that a mismatch is logged. Full suite: 371 passed, 7 skipped.
Note this removes the silent failure mode; the deploy-test half was already fixed in #504.
🤖 Generated with Claude Code