Stop the curriculum schedule starting below min_difficulty - #8334
Stop the curriculum schedule starting below min_difficulty#8334vineethsaivs wants to merge 1 commit into
Conversation
__fixed_root_get_difficulty, which both fixed_linear and fixed_root use, floors
the interpolated difficulty to a multiple of difficulty_step and then clamps only
the top:
next_difficulty -= (next_difficulty % difficulty_step)
next_difficulty = min(next_difficulty, max_difficulty)
The floor has nothing stopping it below the configured start. With
min_difficulty=8 and difficulty_step=16, a pair the tutorial recommends together
for INT8 data on a million-scale model, the first step returns 0, which is a
zero-length sequence for the seqlen metric. min_difficulty=1 with
difficulty_step=8 returns 0 as well, and min_difficulty=10 with difficulty_step=8
starts at 8.
Clamp the bottom the way the top already is. The top clamp already returns
max_difficulty verbatim when it is not a multiple of difficulty_step, so both
endpoints being the configured values rather than multiples of the step is the
behaviour this file already has at one end.
Signed-off-by: Vineeth Sai <vineethsai4444@gmail.com>
| # difficulty_step 16 gives 0 on the first step, which is a zero-length sequence | ||
| # for the seqlen metric. Clamp the bottom the way the top already is; both ends | ||
| # are the configured values rather than multiples of the step. | ||
| next_difficulty = max(next_difficulty, self.state[CURRICULUM_LEARNING_MIN_DIFFICULTY]) |
There was a problem hiding this comment.
I ran this against 6505f2ff in a clean container. The bug reproduces at the merge base 92843ad7: min_difficulty=8 with difficulty_step=16 does start at 0.
The clamp buys that with the other invariant this file documents, in the same pair you picked as motivation. Both constructor branches warn that the difficulty "must be a multiple of this difficulty_step ... (requires multiple of 8 (FP16) or 16 (INT8))", and flooring is what guaranteed it. Clamping up to min_difficulty returns a non-multiple whenever min_difficulty % difficulty_step != 0:
first six difficulties, fixed_linear, max=1024, total_curriculum_step=100
base 92843ad7 head 6505f2ff
min=8 step=16 [0, 16, 16, 32, 48...] [8, 16, 16, 32, 48...] 8 is not a multiple of 16
min=10 step=8 [8, 16, 24, 40, 48...] [10, 16, 24, 40, 48...] 10 is not a multiple of 8
So the million-scale INT8 run in your description gets a first-step seqlen of 8 where the constructor says it needs a multiple of 16. Better than 0, and I am not arguing for the old behaviour. What is worth deciding first is that the difficulty_step % 8 warning does not fire here, since 16 is a multiple of 8, so that user gets a misaligned first step with no diagnostic.
Clamping instead to the smallest multiple at or above min_difficulty keeps both: -(-min_difficulty // difficulty_step) * difficulty_step, which is 16 for the 8/16 pair.
Which contract did you mean to win, "starts at min_difficulty" or "every difficulty is a multiple of difficulty_step"? They cannot both hold when min_difficulty is not a multiple of the step, and test_curriculum_endpoints_are_the_configured_values pins the first.
I only exercised get_difficulty, not a training loop.
Symptom
The curriculum schedule can start below the
min_difficultyit is configured with, and for one pair the tutorial itself recommends it starts at 0.min_difficultydifficulty_step[8, 16, 24, 32, 48, 56][0, 16, 16, 32, 48, 48][64, 64, 80, 80, 96, 112][0, 8, 16, 24, 40, 48][8, 16, 24, 40, 48, 56]min_difficulty=8withdifficulty_step=16is not a contrived pair. The tutorial recommends "starting withmin_difficultyat 8 (million-scale models) or 64 (billion-scale models)" and separately "we usually set [difficulty_step] to 8 (for FP16 data) or 16 (for INT8 data)". A million-scale model on INT8 data lands on exactly that combination, and its first training step gets a sequence length of 0.Root cause
__fixed_root_get_difficulty, which serves bothfixed_linear(root degree 1) andfixed_root, floors the interpolated value to a multiple ofdifficulty_stepand then clamps only the top:At step 0 the interpolation is exactly
min_difficulty, so the floor subtractsmin_difficulty % difficulty_stepand there is nothing to stop it going under. The tutorial's own formula for this schedule is((step/total)**(1/root_degree)) * (max_difficulty - min_difficulty) + min_difficulty, which starts atmin_difficulty.Fix
Clamp the bottom the way the top already is, one line.
This does not introduce a new exception to the "difficulty is a multiple of
difficulty_step" rule: the existing top clamp already returnsmax_difficultyverbatim when it is not a multiple. Withmax_difficulty=1000anddifficulty_step=16the schedule returns 1000, not 992, once it runs out. Both endpoints being the configured values rather than multiples of the step is the behaviour this function already has at one end.__fixed_discrete_get_difficultypicks from an explicit list and is untouched.Test
Two tests in
tests/unit/runtime/test_data_efficiency.py, both plain CPU tests rather thanDistributedTest, sinceCurriculumSchedulerneeds neither an accelerator nor a process group:test_curriculum_never_starts_below_min_difficulty, parametrized overfixed_linearandfixed_rootand over five(min_difficulty, difficulty_step)pairs including the tutorial's own recommendations, checks the first twenty steps stay within[min_difficulty, max_difficulty].test_curriculum_endpoints_are_the_configured_valuespins both ends with amax_difficultythat is not a multiple ofdifficulty_step. Its top-end assertion passes on master too, which is what makes it the control for the argument above.Against master: 7 failed, 4 passed, 6 skipped (
assert 0 >= 8,assert 8 >= 10,assert 0 == 8). With the fix: 11 passed, 6 skipped. The 6 skipped are the file's pre-existingDistributedTestcases, which need 2 GPUs; a pristine checkout reports the same 6 skips and nothing else.yapf and flake8 clean, with yapf making no changes to either file.