Repository navigation
docs(skills): add structured DPA4 workflow support - #5961
SchrodingersCattt wants to merge 38 commits into
Conversation
for more information, see https://pre-commit.ci
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds DPA4/SeZM skills for training, fine-tuning, Python inference, and LAMMPS deployment. It documents ChangesDPA4/SeZM skill workflows
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Updated the description for the deepmd-finetune-dpa4 skill to simplify language and remove references to SeZM. Signed-off-by: Ming-Yu Guo <guomy26@mail2.sysu.edu.cn>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/deepmd-finetune-dpa4/SKILL.md`:
- Around line 92-95: Update the LoRA example reference in the training
instructions to use ../../examples/water/dpa4/lora_ft.json, ensuring it resolves
from the skill directory to the repository-level example while preserving the
existing command guidance.
- Around line 27-36: Update skills/deepmd-finetune-dpa4/SKILL.md at lines 27-36,
49-63, and 104-115, skills/deepmd-train/models/dpa4.md at lines 73-84,
skills/deepmd-python-inference/references/model-artifacts.md at lines 63-70, and
skills/lammps-deepmd/references/model-deployment.md at lines 9-15 and 27-36 so
the selected multi-task branch is propagated through training and freezing
workflows, using --model-branch SELECTED_BRANCH for training and --head
SELECTED_BRANCH for freezing or export commands.
In `@skills/deepmd-python-inference/SKILL.md`:
- Around line 31-35: Restrict .pt2 guidance to inference commands and APIs
verified as supported by integration tests. In
skills/deepmd-python-inference/SKILL.md at lines 31-35, 58-60, and 292-293, and
references/model-artifacts.md at lines 17-22, 39-44, and 57-60, remove or revise
any universal .pt2 loading claims and update dp test or dp eval-desc examples to
supported frozen formats or explicit backend flags when .pt2 is unsupported.
Ensure all documented .pt2 paths are consistent with the installed-version
capabilities.
In `@skills/lammps-deepmd/SKILL.md`:
- Line 6: Update the online execution command in the compatibility metadata to
pin both LAMMPS and DeePMD-kit to the tested compatible versions, and apply the
same pins consistently to every command that invokes them.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f040ef9-3491-40d7-a8a4-f8142ef6b61f
📒 Files selected for processing (10)
doc/agent-skills.mdskills/deepmd-finetune-dpa3/SKILL.mdskills/deepmd-finetune-dpa4/SKILL.mdskills/deepmd-python-inference/SKILL.mdskills/deepmd-python-inference/references/model-artifacts.mdskills/deepmd-train/SKILL.mdskills/deepmd-train/models/dpa4.mdskills/lammps-deepmd/SKILL.mdskills/lammps-deepmd/references/commands-and-workflow.mdskills/lammps-deepmd/references/model-deployment.md
|
Addressed the review findings in four focused commits:
Validation: relative links and JSON examples checked, unsupported |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/deepmd-python-inference/references/model-artifacts.md`:
- Around line 71-77: Update the `dp embed` example to pass `--head
SELECTED_BRANCH` when extracting embeddings from multi-task pretrained
checkpoints, preserving the intended fitting head; alternatively, explicitly
document that the example is single-head-only.
- Around line 54-57: Update the DeepPot.eval and dp test coverage statement in
model-artifacts.md to clarify that atomic virial is available only when the
archive was exported with --atomic-virial, rather than implying it is always
supported; keep the other covered outputs unchanged.
In `@skills/lammps-deepmd/SKILL.md`:
- Around line 48-52: Update the local command-line help instruction in SKILL.md
by removing the `tee /dev/tty` pipeline from the `lmp -h` command, so it writes
directly to standard output and works without a controlling terminal.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: de465679-0ebf-4de7-a9c3-863538ed52fd
📒 Files selected for processing (6)
skills/deepmd-finetune-dpa4/SKILL.mdskills/deepmd-python-inference/SKILL.mdskills/deepmd-python-inference/references/model-artifacts.mdskills/deepmd-train/models/dpa4.mdskills/lammps-deepmd/SKILL.mdskills/lammps-deepmd/references/model-deployment.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/deepmd-train/models/dpa4.md
- skills/deepmd-python-inference/SKILL.md
|
Addressed the latest review comments in two focused commits:
Validation: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5961 +/- ##
==========================================
- Coverage 77.74% 77.50% -0.25%
==========================================
Files 1155 1155
Lines 139649 139649
Branches 5056 5056
==========================================
- Hits 108573 108230 -343
- Misses 29191 29535 +344
+ Partials 1885 1884 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Document how to obtain and verify compatible DPA4 checkpoints, decide between standard fine-tuning and LoRA, inspect active adapter state, distinguish merged best checkpoints, and avoid the incompatible bundled checkpoint/example pairing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/deepmd-finetune-dpa4/SKILL.md`:
- Around line 104-111: Update the checker around model_dict, lora, and the
use_lora diagnostic to inspect each dictionary in multi-task branches for a lora
setting. Treat LoRA as enabled when either the top-level model.lora or any
branch-level lora is present, report that combined status, and reject multi-task
configurations whenever branch-level or top-level LoRA is detected.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c666cc7-3e45-49bc-9ca7-08963dd0c3b3
📒 Files selected for processing (1)
skills/deepmd-finetune-dpa4/SKILL.md
Remove smoke-model-specific discussion and detailed checkpoint inspection scripts, retaining concise general requirements for checkpoint selection and LoRA usage.
Signed-off-by: weiqichen77 <weiqchen@hku.hk>
e188dd4 to
f42fdde
Compare
…nto docs/add-deepmd-dpa4-skill
for more information, see https://pre-commit.ci
|
Added DPA4C training guidance in
Validation: |
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Re-reviewed cbff85a3df36c9e21a22704801f909ffba2ddc5f. The LoRA command now preserves the selected multi-task source branch, and the MatMaster submission/audit scripts involved in my previous findings are no longer part of this PR. I checked the current artifact routing, minimal DPA4 configuration, held-out evaluation commands, freeze policy, and LAMMPS template against the repository interfaces. source/tests/common/test_agent_skills.py: 16 passed. No remaining blocking finding in the current documentation/workflow changes; this validation did not launch training or molecular dynamics.
Coding agent: Codex
Codex version: codex-cli 0.154.0
Model: gpt-6-astra
Reasoning effort: xhigh
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Re-reviewed this exact head because review was explicitly re-requested. The head is unchanged from my prior full review; I re-checked the complete 14-file diff, current resolved review threads, repository guidance, and exact-head CI. The previously identified workflow/correctness issues remain addressed, including multi-task branch/head propagation, LoRA source-head preservation, held-out type-map and completeness checks, DPA4/DPA4C export routing and freeze-time policy, and auditable LAMMPS type mapping/deployment guidance. Exact-head CodeQL, Python, C++, CUDA, C-library build, C++ build, and PyPI workflows are all green. I found no new high-confidence blocking issue.
Agent: ChatGPT
Model: GPT-5.6 Sol
GitHub account: njzjz-bot
Reviewed head: cbff85a
Trigger: scheduled all-PR monitoring
OutisLi
left a comment
OutisLi
left a comment
There was a problem hiding this comment.
Please fix the retained-evaluation output naming before merging. I also found an incomplete configuration example.
- [P1] Use per-system output prefixes that survive suffix replacement. In
skills/deepmd-python-inference/references/held-out-evaluation.md:30-36, prefixes such assystem.000andsystem.001both becomesystem.e.out,system.e_peratom.out, andsystem.f.out, because the actual energy detail writer usesPath.with_suffix(...)(deepmd/infer/model_test/ener.py:163-188). Each separatedp testinvocation starts without append mode, so the second system silently overwrites the first system's evidence.
I reproduced this using the exact-head production writer with reference energies 10 and 11: after writing both systems, only three detail files remained, all containing the second system's data. Please use suffix-free identifiers such as system_000/system_001, or separate per-system directories. The fake-dp test at source/tests/common/test_agent_skills.py:159-171 appends extensions and therefore does not match the real writer. Add a two-system regression using the real naming/writing behavior and verify that both outputs survive.
- [P2] Make the DPA4C JSON complete, or explicitly label it as a fragment. The fenced configuration in
skills/deepmd-train/models/dpa4c.md:23-60omitslearning_rate. It normalizes successfully but fails withKeyError: 'learning_rate'atdeepmd/pt_expt/train/training.py:1850. I reproduced this throughdeepmd.main.main(['--pt-expt', 'train', 'input.json']), changing only the dataset path and bounding the step count. The prose links to a maintained complete example, so an explicit instruction to merge this fragment into that example would also resolve the ambiguity; otherwise include the required section. An executable trainer-initialization check would catch this better than substring assertions.
All 16 focused tests passed, and 11 documented CLI forms parsed successfully. The earlier review fixes are present; these findings concern the remaining output-name collision and configuration-example ambiguity. No new runtime performance concern was found in this documentation/skills change.
for more information, see https://pre-commit.ci
|
Please rebase onto or merge the latest master first to pick up model.preset support, then make presets the recommended/default path for new DPA4 and DPA4C training configurations. Use the v20260911 presets for both families: dpa4--v20260911 and dpa4c--v20260911. The main workflow should be choosing the family/grade and using its preset, rather than manually reconstructing the architecture through a list of descriptor and fitting-network parameters. Replace the first examples accordingly, point the DPA4 workflow to examples/water/dpa4/input_preset.json, and move manual architecture overrides to an advanced section. Explicit descriptor/fitting_net keys override the preset, so simply adding preset while retaining the old full architecture blocks would defeat this change. Apart from the dataset's type_map, the usual model-side additions should be limited to task-specific settings such as descriptor.seed/use_amp, fitting_net.seed/vacuum_ref, and optional descriptor.add_chg_spin_ebd/default_chg_spin. Keep the required data, learning-rate and training configuration outside that model-architecture simplification. Please document the vacuum-reference dependency: enabling fitting_net.vacuum_ref also requires an assigned energy bias via model.preset_out_bias.energy, using isolated-atom energies consistent with the training reference calculation and aligned with type_map. Without an assigned bias, vacuum_ref is disabled. Vacuum referencing and charge/spin conditioning should remain explicit modeling choices; they are not interchangeable with execution flags. For fine-tuning, preserve compatibility with the checkpoint rather than blindly replacing its architecture with a newer preset. Update the example regressions to exercise preset expansion followed by validation for both families, retaining the trainer-initialization check. Validate on the updated branch or merge result containing v20260911 support. |
for more information, see https://pre-commit.ci
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Reviewed the new workflow/documentation delta, examples, regression tests and Read the Docs configuration against the current implementation, starting from cbff85a. No new blocking findings.
The earlier issues are addressed: held-out evaluation now uses suffix-free output prefixes, with a production-writer regression ensuring both systems' retained energy outputs survive. DPA4C snippets are explicitly model sections, with complete training inputs supplied by the maintained example and a trainer-initialization regression. DPA4/DPA4C v20260911 preset names, override semantics and vacuum-reference requirements match implementation; fine-tuning guidance preserves the checkpoint architecture.
Python shard 12 shows all 18 test_agent_skills tests passing (https://github.com/deepmodeling/deepmd-kit/actions/runs/37169987191/job/111340854331). This ran on merge commit f66daf8, containing the reviewed head. Python/C++/CUDA workflow summaries, library/package builds, CodeQL, pre-commit and Read the Docs report success; workflow success alone is not a claim that every label-gated GPU job executed.
Verification was source inspection and existing CI evidence; no independent training, export, molecular-dynamics or documentation build was run. Scope is the branch-owned delta and relevant integration points, not unrelated upstream-merge changes.
This branch has not been deployed
7 participants
Summary
deepmd-finetune-dpa4skill while preservingdeepmd-finetune-dpa3deepmd-train.pt,.pt2,.pth, and.pbinference artifact routinglammps-deepmdDesign
The skills remain organized by user workflow rather than adding one umbrella DPA4 skill. Checkpoint routing is based on stored model metadata instead of filenames. DPA4/SeZM guidance covers PyTorch training and fine-tuning, LoRA, held-out evaluation,
.pt2export, Python inference, and LAMMPS deployment. DPA4C guidance covers the public PyTorch Exportable training, freeze, compression, export-time inference policy, and LAMMPS deployment contracts.The changed skills contain no MatMaster, Bohrium, private image, private library path, or platform-specific helper dependency.
Validation
python -m pytest -q -p no:cacheprovider source/tests/common/test_agent_skills.py: 15 passed, 1 skipped; the skipped normalization test requires a built DeePMD checkoutpython -m black --check source/tests/common/test_agent_skills.pygit diff --checkdoc/model/dpa4c.md, reject the invalid--pt_exptspelling, and preserve the same-node export contract