Skip to content

Commit 016a47f

Browse files
author
Pooya Moradi
committed
Fix RL LR-schedule regression: default schedule length to max_train_steps
PR #4029 made get_optimizer (post_train/rl) size the LR warmup/decay schedule from learning_rate_schedule_steps, with a `<= 0` fallback to max_train_steps for the default (-1) case. That fallback is dead code: MaxTextConfig's validator (set_derived_and_validate_values) rewrites learning_rate_schedule_steps == -1 to `steps` (base.yml default 150_001) before get_optimizer runs. So a default RL run sizes warmup to 0.1 * 150_001 = 15_000 steps; on a 500-step run the LR never finishes warming up and is ~300x too low at the same step. rl.yml and rl_mt_jt.yml inherit these defaults and are affected. RL's run length is max_train_steps (num_batches * num_iterations * train_fraction * num_epoch), not `steps` (a pretraining concept). Default the schedule to max_train_steps and honor learning_rate_schedule_steps only when it diverges from `steps` (the validator makes them equal exactly when the user left it unset), which preserves the deliberate-override capability #4029 added. The change is RL-local; pretrain/SFT/DPO use create_learning_rate_schedule and are unaffected. Adds tests/post_training/unit/rl_lr_schedule_test.py, which builds the config through the real pyconfig path so the validator runs. The existing TestGetOptimizer uses a SimpleNamespace, which bypasses the validator and is why the regression shipped.
1 parent 85690da commit 016a47f

2 files changed

Lines changed: 224 additions & 8 deletions

File tree

src/maxtext/trainers/post_train/rl/utils_rl.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -535,15 +535,25 @@ def check_correctness(extracted_response: str, acceptable_answers: list[str], tm
535535
def get_optimizer(tmvp_config: Any, max_train_steps: int) -> optax.GradientTransformation:
536536
"""Function to obtain an optax optimizer, currently we use adamw.
537537
538-
Schedule shape is controlled by `learning_rate_schedule_steps` when set
539-
(>0); this decouples warmup/decay shape from training length so the same
540-
schedule can be applied across runs of different num_batches. Default
541-
(-1) falls back to `max_train_steps` for backward compatibility — matches
542-
the documented behavior of base.yml's `learning_rate_schedule_steps: -1`
543-
("By default the length of the schedule is set to the number of steps").
538+
The LR schedule length defaults to the actual RL run length
539+
(`max_train_steps` = num_batches * num_iterations * train_fraction *
540+
num_epoch). RL does not use the top-level `steps` (a pretraining concept) for
541+
its run length, so the schedule must track `max_train_steps`, not `steps`.
542+
543+
`learning_rate_schedule_steps` may be set to decouple the schedule shape from
544+
the run length (e.g. to match a fixed schedule across runs with different
545+
num_batches). It is honored only as a deliberate override: the config
546+
validator (`MaxTextConfig.set_derived_and_validate_values`) rewrites
547+
`learning_rate_schedule_steps == -1` to `steps` before this runs, so a value
548+
equal to `steps` means "unset" and falls back to `max_train_steps`; only a
549+
value that differs from `steps` is treated as an explicit schedule length.
544550
"""
545-
schedule_steps = getattr(tmvp_config, "learning_rate_schedule_steps", -1)
546-
if schedule_steps is None or schedule_steps <= 0:
551+
lr_schedule_steps = getattr(tmvp_config, "learning_rate_schedule_steps", -1)
552+
config_steps = getattr(tmvp_config, "steps", -1)
553+
if lr_schedule_steps is not None and lr_schedule_steps > 0 and lr_schedule_steps != config_steps:
554+
# Deliberate decoupling of the schedule length from the run length.
555+
schedule_steps = lr_schedule_steps
556+
else:
547557
schedule_steps = max_train_steps
548558
schedule = optax.schedules.warmup_cosine_decay_schedule(
549559
init_value=0.0,
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Regression tests for the RL learning-rate schedule (CPU-only).
16+
17+
These guard the contract that PR #4029 ("get_optimizer: respect
18+
learning_rate_schedule_steps") was supposed to preserve but silently broke:
19+
20+
With a default RL config (learning_rate_schedule_steps unset == -1) the LR
21+
schedule length must equal the actual training length (max_train_steps), so
22+
warmup completes inside the run.
23+
24+
Why the bug shipped (and why the existing get_optimizer tests miss it):
25+
`tests/post_training/unit/rl_utils_test.py::TestGetOptimizer` builds the config
26+
from a `SimpleNamespace` that does not even carry `learning_rate_schedule_steps`,
27+
so `getattr(cfg, "learning_rate_schedule_steps", -1)` returns -1, the
28+
`schedule_steps <= 0` fallback fires, and the schedule correctly tracks
29+
max_train_steps. The Pydantic validator never runs.
30+
31+
In a real run it DOES run: `MaxTextConfig.set_derived_and_validate_values`
32+
rewrites `learning_rate_schedule_steps == -1 -> steps` (base.yml default 150_001)
33+
*before* get_optimizer is called (configs/types.py). The `<= 0` guard in
34+
get_optimizer therefore can never fire, warmup becomes
35+
`0.1 * 150_001 = 15_000` instead of `0.1 * max_train_steps`, and on a 500-step
36+
run the LR is ~300x too low at the same step.
37+
38+
The only way to catch this is to build the config through the REAL pyconfig
39+
path so the validator runs. That is what these tests do.
40+
"""
41+
42+
import sys
43+
import unittest
44+
45+
import pytest
46+
47+
from maxtext.configs import pyconfig
48+
from maxtext.trainers.post_train.rl import utils_rl
49+
from tests.utils.test_helpers import get_test_config_path
50+
51+
pytestmark = [pytest.mark.post_training]
52+
53+
54+
# Tiny-model overrides known to build a valid MaxTextConfig on CPU without
55+
# network access, mirroring tests/post_training/unit/lora_utils_test.py. The
56+
# model shape is irrelevant here (get_optimizer only reads scalar config
57+
# fields); these just let initialize_pydantic validate quickly.
58+
_MODEL_OVERRIDES = {
59+
"per_device_batch_size": 1.0,
60+
"enable_checkpointing": False,
61+
"base_num_decoder_layers": 1,
62+
"attention": "dot_product",
63+
"max_target_length": 8,
64+
"base_emb_dim": 128,
65+
"base_num_query_heads": 2,
66+
"base_num_kv_heads": 2,
67+
"base_mlp_dim": 256,
68+
"max_prefill_predict_length": 4,
69+
"model_name": "llama2-7b",
70+
"enable_nnx": True,
71+
"pure_nnx_decoder": True,
72+
"override_model_config": True,
73+
"weight_dtype": "bfloat16",
74+
}
75+
76+
77+
def _make_config(**overrides):
78+
"""Build a real MaxTextConfig (runs the Pydantic validator) for RL.
79+
80+
Using initialize_pydantic (not a SimpleNamespace) is the whole point: it is
81+
what promotes learning_rate_schedule_steps == -1 to `steps`.
82+
"""
83+
return pyconfig.initialize_pydantic(
84+
[sys.argv[0], get_test_config_path()],
85+
run_name="rl_lr_schedule_test",
86+
**_MODEL_OVERRIDES,
87+
**overrides,
88+
)
89+
90+
91+
def _max_train_steps(config):
92+
"""Mirror of train_rl.get_max_train_steps.
93+
94+
Inlined rather than imported because importing train_rl pulls the heavy RL
95+
training stack (tunix/vLLM). The formula itself is part of the contract under
96+
test, so a drift here is a meaningful signal.
97+
"""
98+
return int(config.num_batches * config.rl.num_iterations * config.train_fraction * config.num_epoch)
99+
100+
101+
def _effective_lr_at_step(opt, step):
102+
"""Step an inject_hyperparams optimizer `step` times and read the LR it
103+
exposes in opt_state.hyperparams. This is exactly the per-step LR that
104+
tunix's peft_trainer reads and logs, so it reflects what training actually
105+
sees, not a re-derivation of the schedule.
106+
"""
107+
import jax.numpy as jnp # pylint: disable=import-outside-toplevel
108+
109+
params = {"w": jnp.zeros((), dtype=jnp.float32)}
110+
grads = {"w": jnp.zeros((), dtype=jnp.float32)}
111+
state = opt.init(params)
112+
for _ in range(step):
113+
_, state = opt.update(grads, state, params)
114+
return float(state.hyperparams["learning_rate"])
115+
116+
117+
class RLLearningRateScheduleTest(unittest.TestCase):
118+
"""Schedule-shape guards for utils_rl.get_optimizer built on a real config."""
119+
120+
@pytest.mark.cpu_only
121+
def test_default_rl_config_warms_up_within_run(self):
122+
"""REGRESSION GUARD (FAILS on PR #4029, passes once fixed).
123+
124+
With learning_rate_schedule_steps unset (-1) and base.yml's default
125+
`steps` (150_001), the LR must still reach its configured peak by the end
126+
of the intended warmup (0.1 * max_train_steps). On the buggy code the
127+
warmup is sized to 150_001, so the LR is stuck near zero for the whole run.
128+
"""
129+
peak = 3e-6
130+
config = _make_config(
131+
learning_rate=peak,
132+
warmup_steps_fraction=0.1,
133+
gradient_clipping_threshold=0.0,
134+
num_batches=500,
135+
num_epoch=1,
136+
train_fraction=1.0,
137+
steps=150_001, # base.yml default (a pretraining-sized number)
138+
learning_rate_schedule_steps=-1, # "user did not set it" (base.yml default)
139+
)
140+
max_train_steps = _max_train_steps(config)
141+
# Sanity: we are in the regime where the bug shows (run << base `steps`).
142+
self.assertLess(max_train_steps, config.steps)
143+
144+
opt = utils_rl.get_optimizer(config, max_train_steps)
145+
warmup_end = int(config.warmup_steps_fraction * max_train_steps)
146+
lr = _effective_lr_at_step(opt, warmup_end + 5)
147+
# Correct: lr ~= peak. Buggy (#4029): lr ~= peak * warmup_end / 15000,
148+
# i.e. < 1% of peak. A 0.5*peak threshold cleanly separates the two.
149+
self.assertGreaterEqual(
150+
lr,
151+
0.5 * peak,
152+
msg=(
153+
f"LR reached only {lr:.2e} by step {warmup_end + 5} (peak={peak:.2e}). "
154+
"Warmup was sized to base.yml `steps`, not max_train_steps "
155+
f"(={max_train_steps}). learning_rate_schedule_steps in effect="
156+
f"{config.learning_rate_schedule_steps}."
157+
),
158+
)
159+
160+
@pytest.mark.cpu_only
161+
def test_explicit_schedule_steps_decouples_from_run_length(self):
162+
"""FEATURE GUARD (passes before and after the fix).
163+
164+
An explicit learning_rate_schedule_steps must drive the schedule shape
165+
independently of max_train_steps (the capability #4029 was adding). This
166+
ensures the regression fix does not simply delete the feature.
167+
"""
168+
peak = 3e-6
169+
schedule_len = 1000
170+
config = _make_config(
171+
learning_rate=peak,
172+
warmup_steps_fraction=0.1,
173+
gradient_clipping_threshold=0.0,
174+
num_batches=50,
175+
num_epoch=1,
176+
train_fraction=1.0,
177+
learning_rate_schedule_steps=schedule_len, # explicitly set by the user
178+
)
179+
max_train_steps = _max_train_steps(config)
180+
181+
opt = utils_rl.get_optimizer(config, max_train_steps)
182+
# The warmup length must follow the explicit schedule (0.1 * 1000 = 100),
183+
# independent of max_train_steps. Probe at fixed steps so the assertion does
184+
# not depend on num_iterations: early in a 1000-step warmup the LR is still
185+
# small, and by ~100 steps it has reached peak. A fix that wrongly forced
186+
# schedule == max_train_steps (a short warmup) would push LR@20 to ~peak and
187+
# trip the first assertion.
188+
self.assertLessEqual(_effective_lr_at_step(opt, 20), 0.4 * peak)
189+
self.assertGreaterEqual(_effective_lr_at_step(opt, int(0.1 * schedule_len) + 5), 0.9 * peak)
190+
191+
@pytest.mark.cpu_only
192+
def test_validator_overwrites_minus_one_sentinel(self):
193+
"""ROOT-CAUSE characterization (effective-value assertion).
194+
195+
The value get_optimizer reads is NOT the -1 the user left: the validator
196+
promoted it to `steps`. This is precisely why get_optimizer's `<= 0`
197+
fallback is dead code in a real run. (Passes today; documents the seam
198+
that makes test_default_rl_config_warms_up_within_run fail.)
199+
"""
200+
config = _make_config(steps=150_001, learning_rate_schedule_steps=-1)
201+
self.assertNotEqual(config.learning_rate_schedule_steps, -1)
202+
self.assertEqual(config.learning_rate_schedule_steps, config.steps)
203+
204+
205+
if __name__ == "__main__":
206+
unittest.main()

0 commit comments

Comments
 (0)