Skip to content

refactor DeepEval #3213

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
Merged
wanghan-iapcm merged 25 commits into from
Feb 8, 2024
Merged

refactor DeepEval #3213

wanghan-iapcm 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 <jinzhe.zeng@rutgers.edu>
@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 <jinzhe.zeng@rutgers.edu>
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

This comment appears to contain commented-out code.
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

DeepTensor.eval_full returns [tuple of size 3](1) and [tuple of size 5](2).
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

Import of module [deepmd.tf.infer.deep_eval](1) 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

Import of module [deepmd.pt.infer.deep_eval](1) begins an import cycle.
@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 <jinzhe.zeng@rutgers.edu>
@njzjz
fix PT tests
02b3a6d 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
fix FrozenModel
4fa8754 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
add docs
6026ab7 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
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

Import of module [deepmd.infer.deep_tensor](1) 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

Import of module [deepmd.infer.deep_eval](1) 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

Import of module [deepmd.infer.deep_tensor](1) 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

Import of module [deepmd.infer.deep_tensor](1) 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

Import of module [deepmd.infer.deep_pot](1) 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

Import of module [deepmd.infer.deep_pot](1) 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

Import of module [deepmd.infer.deep_wfc](1) 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

Import of module [deepmd.infer.deep_eval](1) 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

Import of module [deepmd.infer.deep_eval](1) 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

Import of module [deepmd.infer.deep_dos](1) 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

Import of module [deepmd.infer.deep_eval](1) begins an import cycle.
wanghan-iapcm
wanghan-iapcm requested changes Feb 3, 2024
View reviewed changes
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 <92130845+wanghan-iapcm@users.noreply.github.com>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
add docs
3516792 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz njzjz requested a review from wanghan-iapcm February 3, 2024 06:58
njzjz added 2 commits February 4, 2024 00:31
@njzjz
pass list[OutputVariableDef] instead of bool
3bc7cb5 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
fix py38 compatibility
acfd367 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
wanghan-iapcm
wanghan-iapcm reviewed Feb 4, 2024
View reviewed changes
wanghan-iapcm
wanghan-iapcm reviewed Feb 4, 2024
View reviewed changes
@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 <jinzhe.zeng@rutgers.edu>
njzjz added 6 commits February 4, 2024 20:46
@njzjz
Merge branch 'devel' into deepeval-refactor
db6ac0d 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
use consistent output name for different backends
be4e42f 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
remove the commented out code
199147c 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
Merge branch 'devel' into deepeval-refactor
355c671
@njzjz
use OutputVariableCategory to check odef
8686fa1 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
fix typo
c0a4251 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@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
@njzjz
improve docs and type hints
d7bf113 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@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_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

DeepDOS.eval returns [tuple of size 1](1) and [tuple of size 2](2).
@njzjz njzjz marked this pull request as draft February 8, 2024 08:55
njzjz added 2 commits February 8, 2024 03:55
@njzjz
Merge branch 'devel' into deepeval-refactor
acc85fb
@njzjz
change the OutputVariableDef argument from differentiable to r_differ…
295f4d7 
…entiable and c_differentiable

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz njzjz marked this pull request as ready for review February 8, 2024 08:56
@njzjz
fix the virial shape since DERV_C changed from (3,3) to (9,)
6b104e3 
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@wanghan-iapcm wanghan-iapcm merged commit c235099 into deepmodeling:devel Feb 8, 2024
@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 <jinzhe.zeng@rutgers.edu>
@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 <jinzhe.zeng@rutgers.edu>
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 <jinzhe.zeng@rutgers.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iProzd iProzd iProzd approved these changes

@wanghan-iapcm wanghan-iapcm wanghan-iapcm 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