Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add DipoleModel and PolarModel #3309

Merged
merged 18 commits into from
Feb 21, 2024
Merged

Conversation

anyangml
Copy link
Collaborator

@anyangml anyangml commented Feb 20, 2024

This PR is to provide model wrappers for DipoleFittingNet and PolarFittingNet, such that the saved model can be used directly in inference with DeepDiole and DeepPolar.

@anyangml anyangml marked this pull request as draft February 20, 2024 07:47
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (4956864) 75.29% compared to head (5f1aec9) 75.31%.

Files Patch % Lines
deepmd/pt/model/model/dipole_model.py 63.88% 13 Missing ⚠️
deepmd/pt/model/model/polar_model.py 70.83% 7 Missing ⚠️
deepmd/pt/model/model/dp_zbl_model.py 42.85% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #3309      +/-   ##
==========================================
+ Coverage   75.29%   75.31%   +0.01%     
==========================================
  Files         398      400       +2     
  Lines       33694    33783      +89     
  Branches     1604     1604              
==========================================
+ Hits        25371    25442      +71     
- Misses       7462     7480      +18     
  Partials      861      861              

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

@anyangml
Copy link
Collaborator Author

@njzjz @wanghan-iapcm , not sure if my OutDef shape is wrong
image
I am expecting 1, 5, 3, 3 and 1, 5, 3, 9.
we may need to change this function to accommodate dipole shape.

Also, we have to add dipole_derv_c otherwise, we have length mismatch here

)
return dict(
zip(
[x.name for x in request_defs],
out,
)
)

@anyangml
Copy link
Collaborator Author

this will return 0, since get_sel_type() returns an empty list in GeneralFitting.

sel_natoms = self._get_sel_natoms(atom_types[0])

@njzjz
Copy link
Member

njzjz commented Feb 20, 2024

@njzjz @wanghan-iapcm , not sure if my OutDef shape is wrong
I am expecting 1, 5, 3, 3 and 1, 5, 3, 9. we may need to change this function to accommodate dipole shape.

We have to use torch.permute to change the output shape, so it can be compatible with the current DPLR code. (obviously, we don't want to rewrite these codes or break the existing TF models)

For how OutDef defines, I will follow @wanghan-iapcm's suggestion.

@njzjz
Copy link
Member

njzjz commented Feb 20, 2024

this will return 0, since get_sel_type() returns an empty list in GeneralFitting.

_get_sel_natoms needs to handle this particular situation.

@anyangml anyangml marked this pull request as ready for review February 21, 2024 02:28
@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 21, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 21, 2024
@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 21, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 21, 2024
@wanghan-iapcm wanghan-iapcm added this pull request to the merge queue Feb 21, 2024
Merged via the queue into deepmodeling:devel with commit af14ba4 Feb 21, 2024
48 checks passed
@njzjz njzjz mentioned this pull request Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants