refact: the DPA1 descriptor#3696
Conversation
![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 #3696 +/- ##
==========================================
+ Coverage 81.89% 82.12% +0.23%
==========================================
Files 509 511 +2
Lines 46591 47364 +773
Branches 2952 2952
==========================================
+ Hits 38156 38899 +743
- Misses 7542 7572 +30
Partials 893 893 ☔ View full report in Codecov by Sentry. |
node_modules/.cache/prettier/.prettier-caches/9be9545d53bb64e65febe2ff48926b4145285f3a.json
Outdated
Show resolved
Hide resolved
wanghan-iapcm
left a comment
wanghan-iapcm
left a comment
There was a problem hiding this comment.
stripped type embedding should be added in a future PR.
![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
Out of diff range and nitpick comments (1)
deepmd/dpmodel/descriptor/dpa1.py (1)
279-285: Initialization ofTypeEmbedNetwith hardcoded activation function 'Linear'. Consider making this configurable if flexibility is required.
There was a problem hiding this comment.
Actionable comments posted: 8
Out of diff range and nitpick comments (3)
deepmd/dpmodel/descriptor/dpa1.py (2)
206-206: Ensure clarity on parameter usage in the constructor.It's important to clearly document which parameters are in use and which are kept for compatibility. This helps maintain clarity and prevent confusion for future developers or users of the API.
614-664: Ensure proper handling of attention layers.When implementing complex layers like
NeighborGatedAttention, it's crucial to ensure that each layer is handled correctly, especially in serialization and deserialization processes to maintain state consistency across sessions.deepmd/tf/descriptor/se_atten.py (1)
3-3: Consider removing unused imports to keep the code clean and maintainable.
iProzd
dismissed
coderabbitai[bot]’s stale review
Some are unnecessary changes requested.
After #3696, we don't need keras anymore. Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
This PR reformat DPA1 descriptor consistent for TF/PT/DP - Clean up the implementation of DPA1 descriptor in TF/PT - Add numpy implementation for DPA1 descriptor - Add `serialize` and `deserialize` for DPA1 descriptor in TF/PT/DP - Add consistence test for TF/PT/DP Now DPA1 descriptor in TF and PT&DP are consistent when: `smooth_type_embdding` == False `env_protection` == 0.0 `stripped_type_embedding` == False DPA1 descriptor in PT and DP are always consistent. Need to fix - consistence in TF: when `excluded_types` != [] and `attn_layer` > 0. --------- Signed-off-by: Duo <50307526+iProzd@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
After deepmodeling#3696, we don't need keras anymore. Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
3 participants
This PR reformat DPA1 descriptor consistent for TF/PT/DP
serializeanddeserializefor DPA1 descriptor in TF/PT/DPNow DPA1 descriptor in TF and PT&DP are consistent when:
smooth_type_embdding== Falseenv_protection== 0.0stripped_type_embedding== FalseDPA1 descriptor in PT and DP are always consistent.
Need to fix
excluded_types!= [] andattn_layer> 0.