add training_reuse_decay_steps by tuoping · Pull Request #1055 · deepmodeling/dpgen

tuoping · 2022-11-29T15:03:04Z

When starting with init-model, add a "training_reuse_decay_steps" parameter.

njzjz · 2022-11-29T23:29:28Z

        if jinput['loss'].get('start_pref_f') is not None:
            jinput['loss']['start_pref_f'] = training_reuse_start_pref_f
        jinput['learning_rate']['start_lr'] = training_reuse_start_lr
+        jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps


This is a breaking change

codecov-commenter · 2022-12-22T04:56:58Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.55%. Comparing base (02d7d7d) to head (47e5733).
⚠️ Report is 309 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1055   +/-   ##
=======================================
  Coverage   39.54%   39.55%           
=======================================
  Files          99       99           
  Lines       17980    17982    +2     
=======================================
+ Hits         7111     7113    +2     
  Misses      10869    10869

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

njzjz-bot

Verdict: Changes requested. The new parameter defaults to None, but the code unconditionally overwrites an existing valid decay_steps value, so every reuse-training configuration that omits the parameter gets decay_steps: null. The PR also lacks schema/docs updates and tests for the default behavior.

Note: The Codex quota is about to reset, so I am using the remaining tokens to review all open PRs in this repository.

Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot · 2026-08-10T14:21:57Z

        if jinput['loss'].get('start_pref_f') is not None:
            jinput['loss']['start_pref_f'] = training_reuse_start_pref_f
        jinput['learning_rate']['start_lr'] = training_reuse_start_lr
+        jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps


[Blocking] training_reuse_decay_steps defaults to None, so this unconditional assignment overwrites an existing integer in the training template and produces decay_steps: null, which DeePMD cannot consume. Override the value only when the user explicitly provides it:

Suggested change

jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

if training_reuse_decay_steps is not None:

jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

Also add arginfo/documentation and tests proving that omission preserves the original value while an explicit value overrides it.

njzjz-bot

Independent review C

REQUEST_CHANGES. Omitting the new optional setting currently overwrites an existing valid learning-rate schedule with JSON null, breaking the default reuse path.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot · 2026-08-29T15:29:48Z

        if jinput['loss'].get('start_pref_f') is not None:
            jinput['loss']['start_pref_f'] = training_reuse_start_pref_f
        jinput['learning_rate']['start_lr'] = training_reuse_start_lr
+        jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps


When training_reuse_decay_steps is omitted, the value above is None, but this assignment still replaces the template's numeric decay_steps with null. DeePMD expects a numeric decay interval, so existing reuse configurations that do not opt into the new key regress. Only override the field when the user provided a value.

Suggested change

jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

if training_reuse_decay_steps is not None:

jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

Fixed in 15fe50a by overriding decay_steps only when training_reuse_decay_steps is provided, preserving the template schedule otherwise. Validation: Python syntax compilation passed. The historical training unittest cannot import in the current environment because this old branch requires the removed dpdata.abacus API.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot

Independent review A

Blocking finding attached inline. Omitting the new option overwrites a valid inherited DeePMD learning-rate setting with JSON null.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot · 2026-08-29T15:38:21Z

        if jinput['loss'].get('start_pref_f') is not None:
            jinput['loss']['start_pref_f'] = training_reuse_start_pref_f
        jinput['learning_rate']['start_lr'] = training_reuse_start_lr
+        jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps


P1: Preserve the existing decay schedule when the optional override is absent. The default above is None, but this unconditional assignment replaces the template's valid decay_steps with null, which is not a usable DeePMD integer setting.

Suggested change

jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

if training_reuse_decay_steps is not None:

jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

Fixed in 15fe50a by overriding decay_steps only when training_reuse_decay_steps is provided, preserving the template schedule otherwise. Validation: Python syntax compilation passed. The historical training unittest cannot import in the current environment because this old branch requires the removed dpdata.abacus API.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot

Independent review B

Requesting changes because omitting the new optional setting overwrites a valid decay schedule with null.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot · 2026-08-29T15:39:07Z

        if jinput['loss'].get('start_pref_f') is not None:
            jinput['loss']['start_pref_f'] = training_reuse_start_pref_f
        jinput['learning_rate']['start_lr'] = training_reuse_start_lr
+        jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps


When training_reuse_decay_steps is absent, this assignment writes None into the DeePMD learning-rate config, replacing the existing numeric decay_steps and producing an invalid training input. Only override the template when the user supplied the new option.\n\nsuggestion\n if training_reuse_decay_steps is not None:\n jinput["learning_rate"]["decay_steps"] = training_reuse_decay_steps\n

Fixed in 15fe50a by overriding decay_steps only when training_reuse_decay_steps is provided, preserving the template schedule otherwise. Validation: Python syntax compilation passed. The historical training unittest cannot import in the current environment because this old branch requires the removed dpdata.abacus API.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh

coderabbitai · 2026-08-29T16:26:36Z

Warning

Review limit reached

Next included review available in 37 minutes.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e0eb920-62ef-44b6-bd11-d0b3d9ac1296

📥 Commits

Reviewing files that changed from the base of the PR and between 02d7d7d and 15fe50a.

📒 Files selected for processing (1)

dpgen/generator/run.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

njzjz-bot

The optional decay-step override was corrected in 15fe50a so the existing schedule is preserved when no override is supplied. The code finding is resolved; the branch still conflicts with master and must be updated before merge.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot

Independent re-review C

No blocking issue found in the patch; the optional override no longer replaces the template decay schedule with None. The PR currently conflicts with the base branch, which remains a merge/rebase limitation.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot

Independent re-review B

The runtime override is useful, but the current head is both conflicting with master and incomplete for DP-GEN's current validated configuration interface. The new key needs to be carried into the current implementation with schema and regression coverage before it is usable.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot · 2026-08-29T16:58:27Z

        training_reuse_stop_batch = 400000

    training_reuse_start_lr = jdata.get('training_reuse_start_lr', 1e-4)
+    training_reuse_decay_steps = jdata.get('training_reuse_decay_steps', None)


Reading this new key only in make_train is insufficient on current DP-GEN: run_jdata_arginfo() performs strict validation, and training_args_dp() does not declare training_reuse_decay_steps. A user adding the documented override will therefore be rejected before this code runs. Please add the optional integer argument (and documentation) to dpgen/generator/arginfo.py, port this change onto current make_train_dp, and add a focused test showing that reuse iterations override learning_rate.decay_steps while the absent key preserves the template value.

njzjz-bot

Independent re-review A

Changes requested at 15fe50a. The None overwrite regression is fixed, but the new public parameter is not represented in the current parameter schema and would be rejected after resolving this branch onto current master. The head also has a merge conflict that must be resolved.

Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

njzjz-bot · 2026-08-29T17:02:11Z

        training_reuse_stop_batch = 400000

    training_reuse_start_lr = jdata.get('training_reuse_start_lr', 1e-4)
+    training_reuse_decay_steps = jdata.get('training_reuse_decay_steps', None)


Please add training_reuse_decay_steps to training_args_dp() in dpgen/generator/arginfo.py when rebasing this change. Current dpgen run input normalization is strict; without a corresponding Argument, users cannot actually set this new key because the CLI rejects it as undefined before make_train_dp() reads it. A schema-normalization test plus a make-train regression for both unset and explicitly set values would keep the intended behavior covered.

njzjz-bot

Reviewed commit 15fe50a66e0748746785e754da27165b657679b0.

Rechecked the optional decay override: omission now preserves the template value, so the earlier null-overwrite defect is fixed. The existing request to declare training_reuse_decay_steps in the current training schema remains unresolved, and the branch conflicts with master. Port it to make_train_dp together with schema coverage when rebasing.

Existing inline discussion: #1055 (comment)

Validation: static review of the guarded assignment and current-base schema. The historical branch has no added regression test, and no full-suite pass is claimed.

Coding agent: Codex
Codex version: codex-cli 0.154.0
Model: gpt-6-astra
Reasoning effort: xhigh

njzjz-bot

The original None-overwrite regression is fixed on this head: the template decay_steps value is now preserved unless an explicit reuse override is provided. However, the remaining current-interface blocker from the existing unresolved inline thread is still valid: training_reuse_decay_steps is a new public configuration key, but current DP-GEN performs strict input normalization and the key is not declared in the training argument schema. After rebasing to current master, users would therefore be rejected before the training code can consume the option.

Please port the override into the current training implementation together with the corresponding schema/documentation entry, and add regression coverage for both omitted and explicitly supplied values. The branch is currently not mergeable against master, and no exact-head workflow runs are available for this SHA, so a rebase and fresh CI are also required. I am not duplicating the existing inline comment.

Agent: ChatGPT
Model: GPT-5.6 Sol
GitHub account: njzjz-bot
Reviewed head: 15fe50a
Trigger: scheduled all-PR monitoring

add reuse_decay_steps

f0ade94

njzjz changed the base branch from master to devel November 29, 2022 23:28

njzjz requested changes Nov 29, 2022

View reviewed changes

Merge branch 'devel' into reuse_decay_steps

47e5733

njzjz-bot suggested changes Aug 10, 2026

View reviewed changes

njzjz-bot suggested changes Aug 29, 2026

View reviewed changes

fix: preserve default reuse decay steps

15fe50a

Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh

njzjz-bot approved these changes Aug 29, 2026

View reviewed changes

njzjz-bot suggested changes Aug 29, 2026

View reviewed changes

njzjz-bot reviewed Sep 12, 2026

View reviewed changes

njzjz-bot suggested changes Sep 21, 2026

View reviewed changes

	jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps
	if training_reuse_decay_steps is not None:
	jinput['learning_rate']['decay_steps'] = training_reuse_decay_steps

Conversation

tuoping commented Nov 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Dec 22, 2022 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

njzjz-bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njzjz-bot Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 29, 2026

Review limit reached

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

njzjz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Dec 22, 2022 •

edited by codecov Bot

Loading

njzjz-bot left a comment •

edited

Loading

njzjz-bot Aug 10, 2026 •

edited

Loading