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

Fix PT DeepPot and replace ASE calculator #3186

Merged
merged 3 commits into from
Jan 27, 2024

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented Jan 27, 2024

  • Set deepmd.pt.utils.ase_calc.DPCalculator as an alias of deepmd.calculator.DP;
  • Replace deepmd_pt with deepmd.pt in deep_pot.py; fix (atomic) virial output shape of DeepPot; add tests for them;
  • Set pbc in pt/test_calculator.py as it requests stress.

- Set `deepmd.pt.utils.ase_calc.DPCalculator` as an alias of `deepmd.calculator.DP`
- Replace `deepmd_pt` with  `deepmd.pt` in `deep_pot.py`
- Set pbc in `pt/test_calculator.py` as it requests stress

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@@ -56,7 +56,7 @@

return super().__new__(DeepPotTF)
elif backend == DPBackend.PyTorch:
from deepmd_pt.infer.deep_eval import DeepPot as DeepPotPT
from deepmd.pt.infer.deep_eval import DeepPot as DeepPotPT

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.pt.infer.deep_eval
begins an import cycle.
@njzjz
Copy link
Member Author

njzjz commented Jan 27, 2024

Set pbc in pt/test_calculator.py as it requests stress

I think it's meaningless to calculate stress for a non-periodic system, right?

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f4d7c7e) 74.29% compared to head (e0cc0bb) 74.32%.
Report is 3 commits behind head on devel.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #3186      +/-   ##
==========================================
+ Coverage   74.29%   74.32%   +0.02%     
==========================================
  Files         343      343              
  Lines       31638    31608      -30     
  Branches     1592     1592              
==========================================
- Hits        23506    23492      -14     
+ Misses       7207     7191      -16     
  Partials      925      925              

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

@wanghan-iapcm
Copy link
Collaborator

Set pbc in pt/test_calculator.py as it requests stress

I think it's meaningless to calculate stress for a non-periodic system, right?

no it does not make sense

@@ -56,7 +56,7 @@ def __new__(cls, model_file: str, *args, **kwargs):

return super().__new__(DeepPotTF)
elif backend == DPBackend.PyTorch:
from deepmd_pt.infer.deep_eval import DeepPot as DeepPotPT
from deepmd.pt.infer.deep_eval import DeepPot as DeepPotPT
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to check with the pt backend in the UTs of python inference

Copy link
Collaborator

@wanghan-iapcm wanghan-iapcm left a comment

Choose a reason for hiding this comment

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

please add the UT as commented.

@njzjz njzjz changed the title Replace deepmd.pt.utils.ase_calc with deepmd.calculator Fix PT DeepPot and replace ASE calculator Jan 27, 2024
njzjz added 2 commits January 27, 2024 05:50
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz njzjz requested a review from wanghan-iapcm January 27, 2024 11:17
@wanghan-iapcm wanghan-iapcm merged commit 3e4715f into deepmodeling:devel Jan 27, 2024
45 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.

2 participants