Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions deepmd/dpmodel/utils/learning_rate.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,3 +692,30 @@ def _decay_value(self, step: int | Array) -> Array:
# Clip to min_lr for steps beyond decay_num_steps
step_lr = xp.where(step >= self.decay_num_steps, min_lr, step_lr)
return step_lr


def make_learning_rate_schedule(
lr_params: dict[str, Any],
num_steps: int,
) -> BaseLR:
"""Build a registered learning-rate schedule for a training run.

The input schema selects schedules through ``learning_rate.type``. Keep
backend trainers on the shared :class:`BaseLR` registry so every backend
accepts the same registered variants without mutating its input config.

Parameters
----------
lr_params : dict[str, Any]
Learning-rate configuration, including the optional ``type`` key.
num_steps : int
Total number of training steps used to parameterize the schedule.

Returns
-------
BaseLR
The schedule selected by ``lr_params["type"]`` (``exp`` by default).
"""
params = dict(lr_params)
params["num_steps"] = num_steps
return BaseLR(**params)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve the optional default learning-rate type

Both the common schema and this helper's docstring define type as optional with exp as the default, but BaseLR.__new__ requires an explicit type. Consequently, make_learning_rate_schedule({"start_lr": 1e-3, "stop_lr": 1e-5}, 100) now raises KeyError: the type of the BaseLR should be set by type. This also regresses the previous direct-trainer behavior: JAX explicitly used lr_param.get("type", "exp"), while pt_expt and TF2 directly constructed LearningRateExp and therefore did not require the key.

The standard CLI normalization path inserts type="exp", so this does not block normalized dp train inputs; it affects direct use of this new helper and direct/backend trainer construction with otherwise valid schema-shaped parameters. Please add params.setdefault("type", "exp") before dispatch and cover the omitted-type case in the factory test.

11 changes: 5 additions & 6 deletions deepmd/jax/train/trainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@
resolve_best_checkpoint_dir,
)
from deepmd.dpmodel.utils.learning_rate import (
LearningRateExp,
BaseLR,
make_learning_rate_schedule,
)
from deepmd.dpmodel.utils.multi_task import (
apply_shared_links,
Expand Down Expand Up @@ -277,11 +278,9 @@ def _deserialize_models(model_data: dict[str, Any]) -> dict[str, BaseModel]:
}
return {DEFAULT_TASK_KEY: BaseModel.deserialize(model_data["model"])}

def _get_lr_and_coef(self, lr_param: dict[str, Any]) -> LearningRateExp:
lr_type = lr_param.get("type", "exp")
if lr_type == "exp":
return LearningRateExp(**lr_param, num_steps=self.num_steps)
raise RuntimeError("unknown learning_rate type " + lr_type)
def _get_lr_and_coef(self, lr_param: dict[str, Any]) -> BaseLR:
"""Construct the schema-selected shared learning-rate schedule."""
return make_learning_rate_schedule(lr_param, self.num_steps)

def _build_losses(
self,
Expand Down
8 changes: 4 additions & 4 deletions deepmd/pt_expt/train/training.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
split_batch,
)
from deepmd.dpmodel.utils.learning_rate import (
LearningRateExp,
make_learning_rate_schedule,
)
from deepmd.pt.train.utils import (
resolve_best_checkpoint_dir,
Expand Down Expand Up @@ -1476,9 +1476,9 @@ def _make_sample(
self.model_prob = None

# Learning rate -------------------------------------------------------
lr_params = config["learning_rate"].copy()
lr_params["num_steps"] = self.num_steps
self.lr_schedule = LearningRateExp(**lr_params)
self.lr_schedule = make_learning_rate_schedule(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep the LambdaLR denominator nonzero for warmup schedules

Dispatching cosine and wsd here routes them into the existing scheduler construction below, which sets initial_lr = float(self.lr_schedule.value(self.start_step)) and divides every lambda value by initial_lr. On a fresh run, the common schema's legal warmup default (warmup_steps > 0, warmup_start_factor=0.0) gives value(0) == 0, so LambdaLR evaluates the lambda during its constructor and immediately raises ZeroDivisionError; no training step runs. I reproduced this after strict argcheck normalization for both newly supported schedule types (and the pre-existing exp path has the same latent defect).

Please use the nonzero lr_schedule.start_lr as the LambdaLR base/denominator and set each optimizer param group's initial_lr accordingly, as legacy PT's _create_lr_scheduler already does. Then lambda(0) can correctly be zero and initialize the optimizer at the intended zero warmup LR without division by zero. Add a real pt_expt Trainer regression with cosine and wsd, warmup_steps > 0, and the default start factor; the current factory-only test cannot exercise this backend integration.

config["learning_rate"], self.num_steps
)

# Gradient clipping
self.gradient_max_norm = training_params.get("gradient_max_norm", 0.0)
Expand Down
8 changes: 4 additions & 4 deletions deepmd/tf2/train/trainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
split_batch,
)
from deepmd.dpmodel.utils.learning_rate import (
LearningRateExp,
make_learning_rate_schedule,
)
from deepmd.dpmodel.utils.training_utils import (
resolve_model_prob,
Expand Down Expand Up @@ -385,9 +385,9 @@ def sample(
resume=init_model is not None or restart_model is not None
)

lr_params = dict(config["learning_rate"])
lr_params["num_steps"] = self.num_steps
self.lr_schedule = LearningRateExp(**lr_params)
self.lr_schedule = make_learning_rate_schedule(
config["learning_rate"], self.num_steps
)
self.optimizer = self._build_optimizer(config.get("optimizer", {}))
self.model_container = _TaskModelContainer(self.models)
self.step = tf.Variable(0, dtype=tf.int64, trainable=False, name="step")
Expand Down
25 changes: 25 additions & 0 deletions source/tests/universal/dpmodel/utils/test_learning_rate.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
to_numpy_array,
)
from deepmd.dpmodel.utils.learning_rate import (
BaseLR,
LearningRateCosine,
LearningRateExp,
LearningRateWSD,
make_learning_rate_schedule,
)


Expand Down Expand Up @@ -369,6 +371,29 @@ def test_array_input_wsd_cosine(self) -> None:
np.testing.assert_allclose(lrs[3], 1e-5, rtol=1e-10)


class TestMakeLearningRateSchedule(unittest.TestCase):
"""Test the shared factory used by backend trainers."""

def test_dispatches_all_schema_variants_without_mutating_config(self) -> None:
"""Select exp, cosine, and WSD schedules through ``type``."""
schedule_types: list[tuple[str, type[BaseLR]]] = [
("exp", LearningRateExp),
("cosine", LearningRateCosine),
("wsd", LearningRateWSD),
]
for schedule_type, expected_class in schedule_types:
with self.subTest(schedule_type=schedule_type):
params = {
"type": schedule_type,
"start_lr": 1e-3,
"stop_lr": 1e-5,
}
schedule = make_learning_rate_schedule(params, num_steps=100)

self.assertIsInstance(schedule, expected_class)
self.assertNotIn("num_steps", params)


class TestLearningRateBeyondStopSteps(unittest.TestCase):
"""Test learning rate behavior beyond num_steps."""

Expand Down
Loading