Skip to content

feat(pt): support shared_dict in linear energy model#5550

Closed
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bothub:fix/issue-4412-linear-shared-dict
Closed

feat(pt): support shared_dict in linear energy model#5550
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bothub:fix/issue-4412-linear-shared-dict

Conversation

@njzjz-bot

@njzjz-bot njzjz-bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Build on the shared_dict idea from feat(pt): implement shared_dict functionality for linear models #4933 for the PyTorch linear_ener model.
  • Preprocess linear-model shared_dict references by adapting the existing multi-task shared-parameter preprocessor to the linear model's models list.
  • Apply descriptor/fitting parameter sharing between linear submodels after data statistics are computed, matching the multi-task timing.
  • Add argument validation support and unit coverage for linear-model shared_dict.

Verification

  • python3 -m py_compile deepmd/pt/utils/multi_task.py deepmd/pt/entrypoints/main.py deepmd/pt/model/model/__init__.py deepmd/pt/model/model/dp_linear_model.py deepmd/pt/train/training.py deepmd/utils/argcheck.py source/tests/pt/model/test_shared_dict_linear.py
  • Could not run the new unittest in this workspace because the local Python environment lacks torch and pytest.

Fixes #4412

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)

Summary by CodeRabbit

  • New Features

    • Linear energy models now support parameter sharing across sub-models through a new shared_dict configuration option, allowing efficient reuse of descriptors and other model components.
  • Tests

    • Added test coverage validating parameter sharing functionality for linear energy models.

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds shared_dict support to the linear_ener model, mirroring multi-task parameter-sharing behaviour. A new preprocess_linear_shared_params adapter converts the models list into a temporary model_dict, reuses preprocess_shared_params, and returns shared_links. LinearEnergyModel.share_params is introduced to apply those links. The training entrypoint and Trainer are updated to invoke this path for single-task linear models, the argcheck schema gains an optional shared_dict field, and two new tests validate the end-to-end flow.

Changes

shared_dict for linear_ener model

Layer / File(s) Summary
Config schema and preprocessing adapter
deepmd/utils/argcheck.py, deepmd/pt/utils/multi_task.py
Adds an optional shared_dict field to linear_ener model args. Introduces preprocess_linear_shared_params, adds require_shared_type_map flag to preprocess_shared_params, and adds an isinstance(item_params, dict) guard for hybrid descriptor routing.
Model factory: preprocess and attach shared_links
deepmd/pt/model/model/__init__.py
get_linear_model calls preprocess_linear_shared_params and stores the returned shared_links on the constructed LinearEnergyModel instance.
LinearEnergyModel.share_params implementation
deepmd/pt/model/model/dp_linear_model.py
New share_params method resolves descriptor and fitting-network modules from shared_links, enforces shared_level ordering, validates class consistency, and delegates to underlying submodule share_params.
Training entrypoint and Trainer wiring
deepmd/pt/entrypoints/main.py, deepmd/pt/train/training.py
Entrypoint preprocesses linear configs and produces shared_links. Trainer.__init__ calls model.share_params for single-task linear models; the existing wrapper.share_params path is now gated to multi-task mode only.
Tests
source/tests/pt/model/test_shared_dict_linear.py
Adds TestSharedDictLinear with tests asserting preprocessing output structure and descriptor type_embedding identity after share_params.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

new feature, Python

Suggested reviewers

  • wanghan-iapcm
  • njzjz
  • anyangml
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(pt): support shared_dict in linear energy model' accurately summarizes the main feature addition across all changed files.
Linked Issues check ✅ Passed The PR implementation fulfills all primary objectives from issue #4412: enables parameter sharing via shared_dict in linear models, supports configuration references as shown in the issue example, and maintains compatibility with the multi-task shared_dict design pattern.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing shared_dict support for linear energy models as specified in issue #4412; no extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@deepmd/pt/model/model/dp_linear_model.py`:
- Around line 86-88: The assertion checking shared_level ordering only compares
each shared_level_link against the shared_level_base, which allows unsorted
sequences like [0, 2, 1] to pass validation. To enforce true monotonic ordering,
you need to track and compare each shared_level_link against the previously
processed shared_level value (not just the base level). Update the assertion
logic in both locations (around line 86-88 and line 103-105) to maintain a
running reference to the last processed shared_level and compare each new link
against that previous value to ensure strictly increasing order.
🪄 Autofix (Beta)

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

Run ID: 0dcd25b7-05b3-49ce-bf5c-3d1192e63e67

📥 Commits

Reviewing files that changed from the base of the PR and between 4a552e3 and 7fde7ec.

📒 Files selected for processing (7)
  • deepmd/pt/entrypoints/main.py
  • deepmd/pt/model/model/__init__.py
  • deepmd/pt/model/model/dp_linear_model.py
  • deepmd/pt/train/training.py
  • deepmd/pt/utils/multi_task.py
  • deepmd/utils/argcheck.py
  • source/tests/pt/model/test_shared_dict_linear.py

Comment on lines +86 to +88
assert shared_level_link >= shared_level_base, (
"The shared_links must be sorted by shared_level!"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enforce true monotonic shared_level ordering.

Line 86 and Line 103 only compare each link against the base level, so an unsorted sequence like 0, 2, 1 still passes. That can apply sharing in the wrong order.

Suggested fix
         for shared_item in shared_links:
             shared_base = shared_links[shared_item]["links"][0]
             class_type_base = shared_base["shared_type"]
             model_idx_base = get_model_index(shared_base["model_key"])
             shared_level_base = int(shared_base["shared_level"])

             if "descriptor" in class_type_base:
                 base_class = get_descriptor(model_idx_base, class_type_base)
+                prev_shared_level = shared_level_base
                 for link_item in shared_links[shared_item]["links"][1:]:
                     class_type_link = link_item["shared_type"]
                     shared_level_link = int(link_item["shared_level"])
-                    assert shared_level_link >= shared_level_base, (
+                    assert shared_level_link >= prev_shared_level, (
                         "The shared_links must be sorted by shared_level!"
                     )
+                    prev_shared_level = shared_level_link
                     assert "descriptor" in class_type_link, (
                         f"Class type mismatched: {class_type_base} vs {class_type_link}!"
                     )
                     link_class = get_descriptor(
                         get_model_index(link_item["model_key"]), class_type_link
@@
             else:
                 base_class = get_fitting_net(model_idx_base, class_type_base)
+                prev_shared_level = shared_level_base
                 for link_item in shared_links[shared_item]["links"][1:]:
                     class_type_link = link_item["shared_type"]
                     shared_level_link = int(link_item["shared_level"])
-                    assert shared_level_link >= shared_level_base, (
+                    assert shared_level_link >= prev_shared_level, (
                         "The shared_links must be sorted by shared_level!"
                     )
+                    prev_shared_level = shared_level_link
                     assert class_type_base == class_type_link, (
                         f"Class type mismatched: {class_type_base} vs {class_type_link}!"
                     )

Also applies to: 103-105

🤖 Prompt for 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.

In `@deepmd/pt/model/model/dp_linear_model.py` around lines 86 - 88, The assertion
checking shared_level ordering only compares each shared_level_link against the
shared_level_base, which allows unsorted sequences like [0, 2, 1] to pass
validation. To enforce true monotonic ordering, you need to track and compare
each shared_level_link against the previously processed shared_level value (not
just the base level). Update the assertion logic in both locations (around line
86-88 and line 103-105) to maintain a running reference to the last processed
shared_level and compare each new link against that previous value to ensure
strictly increasing order.

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 34.32836% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.32%. Comparing base (4a552e3) to head (7fde7ec).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
deepmd/pt/model/model/dp_linear_model.py 2.50% 39 Missing ⚠️
deepmd/pt/utils/multi_task.py 81.25% 3 Missing ⚠️
deepmd/pt/entrypoints/main.py 50.00% 1 Missing ⚠️
deepmd/pt/train/training.py 66.66% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (4a552e3) and HEAD (7fde7ec). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (4a552e3) HEAD (7fde7ec)
54 23
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #5550       +/-   ##
===========================================
- Coverage   82.23%   70.32%   -11.91%     
===========================================
  Files         894      870       -24     
  Lines      102002    96536     -5466     
  Branches     4276     3545      -731     
===========================================
- Hits        83877    67887    -15990     
- Misses      16823    27949    +11126     
+ Partials     1302      700      -602     

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@njzjz-bot

Copy link
Copy Markdown
Contributor Author

Closing this duplicate in favor of #5548, which is farther along (newer updates, an extra follow-up fix commit, and the latest CI run).

Authored by OpenClaw (version: 2026.5.28 e932160, model: custom-chat-jinzhezeng-group/gpt-5.5)

@njzjz-bot njzjz-bot closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Use shared_dict for linear model

1 participant