feat(pt/dp): support three-body type embedding #4066
Conversation
WalkthroughWalkthroughThe recent changes introduce new descriptor classes, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DescriptorManager
participant Descriptor
participant Backend
User->>DescriptorManager: Create Descriptor
DescriptorManager->>Descriptor: Initialize with params
Descriptor->>Backend: Configure for simulation
Backend->>Descriptor: Setup complete
Descriptor->>User: Descriptor ready for use
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedMarkdownlint
Additional comments not posted (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #4066 +/- ##
==========================================
+ Coverage 82.95% 83.02% +0.07%
==========================================
Files 522 524 +2
Lines 51044 51628 +584
Branches 3028 3028
==========================================
+ Hits 42343 42864 +521
- Misses 7756 7816 +60
- Partials 945 948 +3 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
deepmd/dpmodel/descriptor/se_t_tebd.py (1)
139-140: Remove redundant code after raising an exception.The
del spinstatement is unnecessary after raisingNotImplementedError.Consider removing it:
- del spindeepmd/pt/model/descriptor/se_t_tebd.py (1)
154-156: Remove redundant code after raising an exception.The
del spinstatement is unnecessary after raisingNotImplementedError.Consider removing it:
- del spin
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
examples/water/se_e3_tebd/input_torch.json (1)
2-2: Consider removing or expanding the comment.The comment "_comment": "that's all" is repeated multiple times throughout the file. Consider removing it or providing more meaningful comments to enhance clarity.
doc/model/train-se-e3-tebd.md (2)
46-46: Consider using "to" instead of "of".The phrase "as an extra input of the embedding network" might be clearer as "as an extra input to the embedding network."
- The type embeddings of neighboring atoms $\mathcal{A}^j$ and $\mathcal{A}^k$ are added as an extra input of the embedding network. + The type embeddings of neighboring atoms $\mathcal{A}^j$ and $\mathcal{A}^k$ are added as an extra input to the embedding network.Tools
LanguageTool
[uncategorized] ~46-~46: The preposition “to” seems more likely in this position.
Context: ...thcal{A}^k$ are added as an extra input of the embedding network. The notation$:$ ...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
60-60: Avoid using hard tabs in Markdown.Replace hard tabs with spaces to ensure consistent formatting.
- "descriptor": { + "descriptor": {Also applies to: 73-73
Tools
Markdownlint
60-60: Column: 1
Hard tabs(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range, codebase verification and nitpick comments (1)
doc/model/train-se-e3-tebd.md (1)
62-75: Replace hard tabs with spaces in JSON configuration.The JSON configuration example uses hard tabs, which should be replaced with spaces to adhere to Markdownlint rules and maintain consistency.
Use this diff to fix the issue:
- "descriptor": { + "descriptor": {Tools
Markdownlint
62-62: Column: 1
Hard tabs(MD010, no-hard-tabs)
75-75: Column: 1
Hard tabs(MD010, no-hard-tabs)
As discussed in deepmodeling#4066 Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
As discussed in #4066 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Updated the description of the `se_e3` notation for improved clarity regarding its relation to three-body embedding in DeepPot-SE. - Enhanced explanation of bond-angle information in the context of the embedding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu> Co-authored-by: Han Wang <92130845+wanghan-iapcm@users.noreply.github.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new descriptor classes `DescrptSeTTebd` and `DescrptBlockSeTTebd` for enhanced molecular simulation capabilities. - Added advanced configuration options with `descrpt_se_e3_tebd_args` to customize descriptor behavior. - Launched the new `"se_e3_tebd"` descriptor for improved atomic configuration representation. - **Bug Fixes** - Improved integration of new descriptors into existing test frameworks, ensuring consistent functionality across multiple backends. - **Tests** - Implemented a comprehensive testing suite for `DescrptSeTTebd`, validating its performance across various configurations and frameworks. - **Documentation** - Updated public API documentation to include newly added descriptor classes and parameters for clarity and usability. - Created a detailed guide for implementing the `se_e3_tebd` descriptor in training models. - **Chores** - Added a structured configuration file `input_torch.json` for model training and evaluation. - Expanded test examples with the inclusion of a new input file path. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Han Wang <92130845+wanghan-iapcm@users.noreply.github.com>
As discussed in deepmodeling#4066 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Updated the description of the `se_e3` notation for improved clarity regarding its relation to three-body embedding in DeepPot-SE. - Enhanced explanation of bond-angle information in the context of the embedding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu> Co-authored-by: Han Wang <92130845+wanghan-iapcm@users.noreply.github.com>
Summary by CodeRabbit
New Features
DescrptSeTTebdandDescrptBlockSeTTebdfor enhanced molecular simulation capabilities.descrpt_se_e3_tebd_argsto customize descriptor behavior."se_e3_tebd"descriptor for improved atomic configuration representation.Bug Fixes
Tests
DescrptSeTTebd, validating its performance across various configurations and frameworks.Documentation
se_e3_tebddescriptor in training models.Chores
input_torch.jsonfor model training and evaluation.