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.

Sign up for GitHub

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

Jump to bottom

refactor DeepEval #3213

Merged
merged 25 commits into from
Feb 8, 2024
Merged

refactor DeepEval #3213

merged 25 commits into from
Feb 8, 2024

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented Feb 1, 2024 •
edited
Loading

Implement high-level DeepEval and low-level DeepEvalBackend.

Ugly things:
(1) DipoleChargeModifier is not updated in this PR. Thus, it still inherits from the old DeepEval. (It should not inherit from DeepEval!!!)
(2) There are no unit tests or testing models for DeepGlobarPolar or DeepWFC.
(3) The shape of the atomic tensor looks different from forces and atomic virials.

TODO:

  • Add docs

@njzjz
refactor DeepEval
e713910 
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz requested review from iProzd and wanghan-iapcm February 1, 2024 13:23
@github-actions github-actions bot added the Python label Feb 1, 2024
@njzjz
fix cycle import
b428fc2 
Signed-off-by: Jinzhe Zeng <[email protected]>
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 1, 2024
View reviewed changes
deepmd/tf/infer/deep_eval.py Outdated
Comment on lines 474 to 475
# for idx,ii in enumerate(imap) :
# ret[:,ii,:] = vec[:,idx,:]

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
deepmd/tf/infer/deep_eval.py Fixed Show fixed Hide fixed
deepmd/infer/deep_dos.py Fixed Show fixed Hide fixed
deepmd/infer/deep_tensor.py
Comment on lines +93 to +103
def eval_full(
self,
coords: np.ndarray,
cells: Optional[np.ndarray],
atom_types: np.ndarray,
atomic: bool = False,
fparam: Optional[np.ndarray] = None,
aparam: Optional[np.ndarray] = None,
mixed_type: bool = False,
**kwargs: dict,
) -> Tuple[np.ndarray, ...]:

Check notice

Code scanning / CodeQL

Returning tuples with varying lengths Note

DeepTensor.eval_full returns
tuple of size 3
Loading
and
tuple of size 5
Loading
.
deepmd/pt/infer/deep_eval.py Fixed Show fixed Hide fixed
deepmd/infer/deep_eval.py
if cls is DeepEvalBase:
backend = detect_backend(model_file)
if backend == DPBackend.TensorFlow:
from deepmd.tf.infer.deep_eval import DeepEval as DeepEvalTF

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.tf.infer.deep_eval
Loading
begins an import cycle.
deepmd/infer/deep_eval.py

return super().__new__(DeepEvalTF)
elif backend == DPBackend.PyTorch:
from deepmd.pt.infer.deep_eval import DeepEval as DeepEvalPT

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.pt.infer.deep_eval
Loading
begins an import cycle.
deepmd/infer/deep_polar.py Fixed Show fixed Hide fixed
deepmd/infer/deep_tensor.py Fixed Show fixed Hide fixed
deepmd/infer/deep_wfc.py Fixed Show fixed Hide fixed
@codecov Codecov
Copy link

codecov bot commented Feb 1, 2024 •
edited
Loading

Codecov Report

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

Comparison is base (cfdda1d) 75.32% compared to head (6b104e3) 75.19%.

Files Patch % Lines
deepmd/tf/infer/deep_eval.py 87.50% 49 Missing ⚠️
deepmd/infer/deep_eval.py 92.98% 8 Missing ⚠️
deepmd/pt/infer/deep_eval.py 86.20% 8 Missing ⚠️
deepmd/tf/entrypoints/test.py 58.82% 7 Missing ⚠️
deepmd/__init__.py 33.33% 2 Missing ⚠️
deepmd/infer/deep_polar.py 84.61% 2 Missing ⚠️
deepmd/infer/deep_dos.py 94.73% 1 Missing ⚠️
deepmd/infer/deep_wfc.py 80.00% 1 Missing ⚠️
deepmd/tf/infer/__init__.py 80.00% 1 Missing ⚠️
deepmd/tf/model/frozen.py 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##            devel    #3213      +/-   ##
==========================================
- Coverage   75.32%   75.19%   -0.13%     
==========================================
  Files         366      372       +6     
  Lines       32966    33136     +170     
  Branches     1608     1608              
==========================================
+ Hits        24830    24916      +86     
- Misses       7263     7347      +84     
  Partials      873      873

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

njzjz added 4 commits February 1, 2024 15:57
@njzjz
fix errors
eb0b7d2 
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz
fix PT tests
02b3a6d 
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz
fix FrozenModel
4fa8754 
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz
add docs
6026ab7 
Signed-off-by: Jinzhe Zeng <[email protected]>
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 1, 2024
View reviewed changes
deepmd/infer/deep_dipole.py
Comment on lines +2 to +4
from deepmd.infer.deep_tensor import (
DeepTensor,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_tensor
Loading
begins an import cycle.
deepmd/infer/deep_dos.py
Comment on lines +17 to +19
from .deep_eval import (
DeepEval,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_eval
Loading
begins an import cycle.
deepmd/infer/deep_polar.py
Comment on lines +8 to +10
from deepmd.infer.deep_tensor import (
DeepTensor,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_tensor
Loading
begins an import cycle.
deepmd/infer/deep_wfc.py
Comment on lines +2 to +4
from deepmd.infer.deep_tensor import (
DeepTensor,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_tensor
Loading
begins an import cycle.
@njzjz njzjz added the Test CUDA Trigger test CUDA workflow label Feb 2, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 2, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 2, 2024
View reviewed changes
deepmd/pt/infer/deep_eval.py
Comment on lines +23 to +25
from deepmd.infer.deep_pot import (
DeepPot,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_pot
Loading
begins an import cycle.
deepmd/tf/infer/deep_eval.py
Comment on lines +37 to +39
from deepmd.infer.deep_pot import (
DeepPot,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_pot
Loading
begins an import cycle.
deepmd/tf/infer/deep_eval.py
Comment on lines +40 to +42
from deepmd.infer.deep_wfc import (
DeepWFC,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_wfc
Loading
begins an import cycle.
deepmd/infer/deep_tensor.py
Comment on lines +17 to +19
from deepmd.infer.deep_eval import (
DeepEval,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_eval
Loading
begins an import cycle.
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 2, 2024
View reviewed changes
deepmd/pt/infer/deep_eval.py
Comment on lines +20 to +22
from deepmd.infer.deep_eval import (
DeepEvalBackend,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_eval
Loading
begins an import cycle.
deepmd/tf/infer/deep_eval.py
Comment on lines +27 to +29
from deepmd.infer.deep_dos import (
DeepDOS,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_dos
Loading
begins an import cycle.
deepmd/tf/infer/deep_eval.py
Comment on lines +30 to +32
from deepmd.infer.deep_eval import (
DeepEvalBackend,
)

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
deepmd.infer.deep_eval
Loading
begins an import cycle.
wanghan-iapcm
wanghan-iapcm requested changes Feb 3, 2024
View reviewed changes
deepmd/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/pt/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/pt/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/pt/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/pt/infer/deep_eval.py Outdated Show resolved Hide resolved
njzjz and others added 2 commits February 3, 2024 01:42
@njzjz @wanghan-iapcm
make atom_types to be int32
51b5e7b 
Co-authored-by: Han Wang <[email protected]>
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz
add docs
3516792 
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz requested a review from wanghan-iapcm February 3, 2024 06:58
@njzjz njzjz mentioned this pull request Feb 4, 2024
Merged
wanghan-iapcm pushed a commit that referenced this pull request Feb 5, 2024
@njzjz
pt: rename atomic_virial to atom_virial in the model output (#3226)
17f2c35 
To be consistent with TF, as discussed in
#3213 (comment).
Old PT models are expected to be incompatible.

Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz requested a review from wanghan-iapcm February 6, 2024 05:20
wanghan-iapcm
wanghan-iapcm approved these changes Feb 7, 2024
View reviewed changes
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.

LGTM

iProzd
iProzd reviewed Feb 8, 2024
View reviewed changes
deepmd/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/infer/deep_pot.py Outdated Show resolved Hide resolved
deepmd/tf/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/tf/infer/deep_eval.py Outdated Show resolved Hide resolved
deepmd/infer/deep_eval.py Outdated Show resolved Hide resolved
@njzjz
improve docs and type hints
d7bf113 
Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz requested a review from iProzd February 8, 2024 08:47
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 8, 2024
View reviewed changes
deepmd/infer/deep_pot.py Fixed Show fixed Hide fixed
deepmd/infer/deep_tensor.py Fixed Show fixed Hide fixed
deepmd/infer/deep_dos.py
Comment on lines +59 to +69
def eval(
self,
coords: np.ndarray,
cells: Optional[np.ndarray],
atom_types: Union[List[int], np.ndarray],
atomic: bool = False,
fparam: Optional[np.ndarray] = None,
aparam: Optional[np.ndarray] = None,
mixed_type: bool = False,
**kwargs: Dict[str, Any],
) -> Tuple[np.ndarray, ...]:

Check notice

Code scanning / CodeQL

Returning tuples with varying lengths Note

DeepDOS.eval returns
tuple of size 1
Loading
and
tuple of size 2
Loading
.
@njzjz njzjz marked this pull request as draft February 8, 2024 08:55
@njzjz njzjz marked this pull request as ready for review February 8, 2024 08:56
@wanghan-iapcm wanghan-iapcm merged commit c235099 into deepmodeling:devel Feb 8, 2024
46 checks passed
@njzjz njzjz mentioned this pull request Feb 9, 2024
Merged
@njzjz njzjz mentioned this pull request Apr 2, 2024
Closed
njzjz added a commit to njzjz/deepmd-kit that referenced this pull request Jun 4, 2024
@njzjz
fix(tf): fix modifier_type in DeepEval
f9e0a5a 
A downgrade in deepmodeling#3213.

Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz mentioned this pull request Jun 4, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 6, 2024
@njzjz
fix(tf): fix modifier_type in DeepEval (#3855)
b780108 
A downgrade in #3213.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Added support for `modifier_type` in the evaluation process to enhance
model flexibility.
- **Tests**
- Introduced unit tests for deep potential model evaluation using
TensorFlow.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Jinzhe Zeng <[email protected]>
mtaillefumier pushed a commit to mtaillefumier/deepmd-kit that referenced this pull request Sep 18, 2024
@njzjz
fix(tf): fix modifier_type in DeepEval (deepmodeling#3855)
26bce1e 
A downgrade in deepmodeling#3213.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Added support for `modifier_type` in the evaluation process to enhance
model flexibility.
- **Tests**
- Introduced unit tests for deep potential model evaluation using
TensorFlow.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Jinzhe Zeng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wanghan-iapcm wanghan-iapcm wanghan-iapcm approved these changes

@iProzd iProzd iProzd approved these changes

Assignees
No one assigned
Labels
Python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@njzjz @iProzd @wanghan-iapcm