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

[Feature Request] Support eval_descriptor for PT DeepEval #4112

Closed
njzjz opened this issue Sep 9, 2024 · 2 comments · Fixed by #4214
Closed

[Feature Request] Support eval_descriptor for PT DeepEval #4112

njzjz opened this issue Sep 9, 2024 · 2 comments · Fixed by #4214
Assignees
Milestone

Comments

@njzjz
Copy link
Member

njzjz commented Sep 9, 2024

Summary

Support eval_descriptor for PT DeepEval.

Detailed Description

The abstract method is here:

def eval_descriptor(
self,
coords: np.ndarray,
cells: Optional[np.ndarray],
atom_types: np.ndarray,
fparam: Optional[np.ndarray] = None,
aparam: Optional[np.ndarray] = None,
efield: Optional[np.ndarray] = None,
mixed_type: bool = False,
**kwargs: Any,
) -> np.ndarray:
"""Evaluate descriptors by using this DP.
Parameters
----------
coords
The coordinates of atoms.
The array should be of size nframes x natoms x 3
cells
The cell of the region.
If None then non-PBC is assumed, otherwise using PBC.
The array should be of size nframes x 9
atom_types
The atom types
The list should contain natoms ints
fparam
The frame parameter.
The array can be of size :
- nframes x dim_fparam.
- dim_fparam. Then all frames are assumed to be provided with the same fparam.
aparam
The atomic parameter
The array can be of size :
- nframes x natoms x dim_aparam.
- natoms x dim_aparam. Then all frames are assumed to be provided with the same aparam.
- dim_aparam. Then all frames and atoms are provided with the same aparam.
efield
The external field on atoms.
The array should be of size nframes x natoms x 3
mixed_type
Whether to perform the mixed_type mode.
If True, the input data has the mixed_type format (see doc/model/train_se_atten.md),
in which frames in a system may have different natoms_vec(s), with the same nloc.
Returns
-------
descriptor
Descriptors.
"""
raise NotImplementedError

However, how to do it for PT needs to be clarified. PT descriptor has a different forward method.

Further Information, Files, and Links

No response

@Chengqian-Zhang
Copy link
Collaborator

I implemented this function some time ago by copying all the forward related code before descriptor to eval_descriptor, but I think this implementation is rather redundant.

@njzjz njzjz added this to the v3.0.0 milestone Sep 26, 2024
@njzjz
Copy link
Member Author

njzjz commented Sep 30, 2024

    def fail(self, *args, **kwargs):
>       raise RuntimeError(name + " is not supported on ScriptModules")
E       RuntimeError: register_forward_hook is not supported on ScriptModules

../../anaconda3/lib/python3.10/site-packages/torch/jit/_script.py:981: RuntimeError

njzjz added a commit to njzjz/deepmd-kit that referenced this issue Oct 14, 2024
@njzjz njzjz linked a pull request Oct 14, 2024 that will close this issue
github-merge-queue bot pushed a commit that referenced this issue Oct 15, 2024
Fix #4112.

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

- **New Features**
- Introduced a method for evaluating descriptors using the deep
potential model.
- Added functionality to control descriptor evaluation during model
operations.

- **Bug Fixes**
- Removed conditional skip for descriptor evaluation tests, enhancing
test coverage for PyTorch models.

- **Tests**
- Added a new test class for neighbor list setups in descriptor
evaluation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
@njzjz njzjz closed this as completed Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants