deepmodeling / deepmd-kit

A deep learning package for many-body potential energy representation and molecular dynamics
https://docs.deepmodeling.com/projects/deepmd/
GNU Lesser General Public License v3.0
1.41k stars 487 forks source link

chore: improve type anotations in deepmd.infer #3792

Closed njzjz closed 1 month ago

njzjz commented 2 months ago

Fix several incorrect type anotations.

Summary by CodeRabbit

These changes aim to improve the overall usability and robustness of the application, making it more adaptable to different scenarios.

coderabbitai[bot] commented 2 months ago
Walkthrough ## Walkthrough The updates primarily focus on enhancing flexibility and type safety across various modules in the `deepmd` package. Key changes include altering function signatures to use more generic type annotations, adding optional parameters, and introducing overloaded methods to support different input scenarios. These modifications aim to make the codebase more adaptable and maintainable, ensuring it can handle a wider range of input types and configurations. ## Changes | File(s) | Change Summary | |---------|----------------| | `deepmd/dpmodel/infer/deep_eval.py`, `deepmd/infer/deep_eval.py`, `deepmd/pt/infer/deep_eval.py`, `deepmd/tf/infer/deep_eval.py` | Updated function signatures to use more generic type annotations (`Any`), made certain parameters optional, and adjusted method return types. | | `deepmd/entrypoints/test.py` | Replaced specific imports with a more general import statement for `DeepTensor`. | | `deepmd/infer/deep_dos.py`, `deepmd/infer/deep_polar.py` | Changed `kwargs` parameter type to `Any`. | | `deepmd/infer/deep_pot.py` | Added overloaded `eval` methods to support different input scenarios based on the `atomic` parameter. | | `deepmd/infer/model_devi.py` | Modified the `calc_model_devi_f` function's signature to include overloads for the `atomic` parameter. | | `deepmd/tf/infer/deep_tensor.py` | Updated `eval` and `eval_full` methods to accept an optional `cells` parameter. |

Recent Review Details **Configuration used: CodeRabbit UI** **Review profile: CHILL**
Commits Files that changed from the base of the PR and between d62a41f37388acd2b09e13a7554b68617612c4c2 and 81ec3b613066f6cb5d10882a4eb479cfa0bccfb2.
Files selected for processing (10) * deepmd/dpmodel/infer/deep_eval.py (2 hunks) * deepmd/entrypoints/test.py (1 hunks) * deepmd/infer/deep_dos.py (2 hunks) * deepmd/infer/deep_eval.py (7 hunks) * deepmd/infer/deep_polar.py (1 hunks) * deepmd/infer/deep_pot.py (3 hunks) * deepmd/infer/model_devi.py (2 hunks) * deepmd/pt/infer/deep_eval.py (4 hunks) * deepmd/tf/infer/deep_eval.py (5 hunks) * deepmd/tf/infer/deep_tensor.py (2 hunks)
Files skipped from review due to trivial changes (2) * deepmd/infer/deep_dos.py * deepmd/infer/deep_polar.py
Additional Context Used
GitHub Check Runs (1)
CodeQL neutral (1)
deepmd/infer/model_devi.py: [notice] 50-50: Statement has no effect This statement has no effect.
Additional comments not posted (26)
deepmd/infer/deep_pot.py (4)
`93-105`: The overloaded method `eval` for `atomic: Literal[True]` is well-defined with appropriate type annotations and optional parameters. --- `107-119`: The overloaded method `eval` for `atomic: Literal[False]` is correctly updated to enhance flexibility and maintainability. --- `121-133`: The addition of a generic overloaded `eval` method with a boolean `atomic` parameter increases the method's flexibility in handling different scenarios. --- Line range hint `134-144`: The main `eval` method implementation is comprehensive, handling various scenarios effectively with the new flexible type annotations.
deepmd/dpmodel/infer/deep_eval.py (2)
`79-82`: The constructor `__init__` now uses more generic type annotations for `*args` and `**kwargs`, enhancing flexibility and maintainability. --- `164-169`: The `eval` method is updated to allow `cells` to be optional and to accept more generic keyword arguments, improving the method's usability in different contexts.
deepmd/tf/infer/deep_tensor.py (2)
`149-149`: The `eval` method now correctly allows the `cells` parameter to be optional, aligning with the changes aimed at increasing method flexibility. --- `279-279`: The `eval_full` method has been updated to allow optional `cells`, maintaining consistency with the `eval` method and enhancing flexibility.
deepmd/infer/model_devi.py (3)
`31-31`: The overloaded method `calc_model_devi_f` for `atomic: Literal[False]` is correctly updated to use a more flexible type annotation. --- `40-40`: The overloaded method `calc_model_devi_f` for `atomic: Literal[True]` is appropriately updated to enhance flexibility. --- `44-50`: The addition of a generic overloaded `calc_model_devi_f` method with a boolean `atomic` parameter increases the method's flexibility in handling different scenarios.
deepmd/infer/deep_eval.py (7)
`86-89`: Updated type annotations for `*args` and `**kwargs` to `Any` enhance flexibility. --- `103-108`: The `cells` parameter is now optional, aligning with the flexibility described in the PR summary. --- `170-176`: The `cells` parameter in `eval_descriptor` method is now optional, which is consistent with the changes in `eval`. --- `250-250`: The method `_check_mixed_types` now returns a boolean directly using `.item()`, which is a more explicit conversion from a numpy array to a native Python type. --- `254-254`: The return type of `model_type` has been updated to `Type["DeepEval"]`, improving type hinting and readability. --- `320-323`: The constructor of `DeepEval` class now uses `Any` for `*args` and `**kwargs`, aligning with the changes in `DeepEvalBackend`. --- `391-391`: The `**kwargs` parameter in `eval_descriptor` of `DeepEval` class is now more generic, which is consistent with other changes in the file.
deepmd/pt/infer/deep_eval.py (3)
`91-95`: Updated type annotations for `*args` and `**kwargs` to `Any` enhance flexibility in the PyTorch implementation of `DeepEval`. --- `169-169`: The return type of `model_type` has been updated to `Type["DeepEvalWrapper"]`, improving type hinting and readability in the PyTorch context. --- `215-220`: The `cells` parameter is now optional in the `eval` method, aligning with changes in the TensorFlow implementation and enhancing flexibility.
deepmd/entrypoints/test.py (1)
`49-49`: Import of `DeepTensor` aligns with the PR's objective to adjust imports for improved type annotations.
deepmd/tf/infer/deep_eval.py (4)
`13-13`: Updated import of `Type` from `typing` module aligns with the new type annotations used in the class. --- `266-266`: Updated return type for `model_type` method to `Type["DeepEvalWrapper"]` enhances type hinting and readability. --- `697-703`: The method signature for `eval` has been updated to use `Optional[np.ndarray]` for `cells` and `Any` for `**kwargs`. This change increases the flexibility of the method by allowing `cells` to be optional and supporting a wider range of keyword arguments. --- Line range hint `1027-1084`: The method signature for `eval_descriptor` and its inner method `_eval_descriptor_inner` have been updated similarly to `eval`, using `Optional[np.ndarray]` for `cells` and allowing more flexible keyword arguments with `Any`. This consistency in method signatures across the class is beneficial for maintainability.
--- Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 82.58%. Comparing base (d62a41f) to head (81ec3b6).

Files Patch % Lines
deepmd/infer/deep_pot.py 66.66% 3 Missing :warning:
deepmd/entrypoints/test.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## devel #3792 +/- ## ========================================== - Coverage 82.58% 82.58% -0.01% ========================================== Files 515 515 Lines 48796 48806 +10 Branches 2982 2982 ========================================== + Hits 40300 40308 +8 - Misses 7585 7587 +2 Partials 911 911 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wanghan-iapcm commented 1 month ago

ref: https://stackoverflow.com/questions/37031928/type-annotations-for-args-and-kwargs