-
Notifications
You must be signed in to change notification settings - Fork 633
fix(train): dispatch registered learning rate schedules #5776
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -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( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Keep the LambdaLR denominator nonzero for warmup schedules Dispatching Please use the nonzero |
||
| config["learning_rate"], self.num_steps | ||
| ) | ||
|
|
||
| # Gradient clipping | ||
| self.gradient_max_norm = training_params.get("gradient_max_norm", 0.0) | ||
|
|
||
There was a problem hiding this comment.
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
typeas optional withexpas the default, butBaseLR.__new__requires an explicittype. Consequently,make_learning_rate_schedule({"start_lr": 1e-3, "stop_lr": 1e-5}, 100)now raisesKeyError: the type of the BaseLR should be set by type. This also regresses the previous direct-trainer behavior: JAX explicitly usedlr_param.get("type", "exp"), while pt_expt and TF2 directly constructedLearningRateExpand therefore did not require the key.The standard CLI normalization path inserts
type="exp", so this does not block normalizeddp traininputs; it affects direct use of this new helper and direct/backend trainer construction with otherwise valid schema-shaped parameters. Please addparams.setdefault("type", "exp")before dispatch and cover the omitted-type case in the factory test.