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

chore: improve type anotations in deepmd.infer #3792

Merged
merged 1 commit into from
May 17, 2024

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented May 17, 2024

Fix several incorrect type anotations.

Summary by CodeRabbit

  • New Features

    • Enhanced flexibility in function and method parameters, allowing for more versatile use cases.
  • Improvements

    • Streamlined type annotations for improved code maintainability and readability.
    • Updated import statements for better module organization and efficiency.
  • Bug Fixes

    • Corrected parameter types to ensure proper handling of optional and varied input types.

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

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Copy link

coderabbitai bot commented May 17, 2024

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 d62a41f and 81ec3b6.
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
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • 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 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 for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

real_f: Optional[np.ndarray] = None,
relative: Optional[float] = None,
atomic: bool = False,
) -> Tuple[np.ndarray, ...]: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.
Copy link

codecov bot commented May 17, 2024

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 ⚠️
deepmd/entrypoints/test.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            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              

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

@njzjz njzjz requested a review from wanghan-iapcm May 17, 2024 05:07
@wanghan-iapcm
Copy link
Collaborator

@wanghan-iapcm wanghan-iapcm added this pull request to the merge queue May 17, 2024
Merged via the queue into deepmodeling:devel with commit 42724ce May 17, 2024
60 checks passed
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.

None yet

2 participants