chore(pt): move deepmd.pt.infer.deep_eval.eval_model to tests
#4153
Conversation
Per discussion in deepmodeling#4142 (comment). It should not be a public API as it lacks maintainance. Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
njzjz
changed the title
deepmd.pt.infer.deep_eval.eval_model to testsdeepmd.pt.infer.deep_eval.eval_model to tests
WalkthroughWalkthroughThe changes involve the removal of the Changes
Possibly related PRs
Suggested reviewers
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedRuff
Additional comments not posted (2)
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- deepmd/pt/infer/deep_eval.py (0 hunks)
- source/tests/pt/common.py (2 hunks)
- source/tests/pt/model/test_autodiff.py (1 hunks)
- source/tests/pt/model/test_forward_lower.py (1 hunks)
- source/tests/pt/model/test_null_input.py (1 hunks)
- source/tests/pt/model/test_permutation.py (1 hunks)
- source/tests/pt/model/test_permutation_denoise.py (1 hunks)
- source/tests/pt/model/test_rot.py (1 hunks)
- source/tests/pt/model/test_rot_denoise.py (1 hunks)
- source/tests/pt/model/test_smooth.py (1 hunks)
- source/tests/pt/model/test_smooth_denoise.py (1 hunks)
- source/tests/pt/model/test_trans.py (1 hunks)
- source/tests/pt/model/test_trans_denoise.py (1 hunks)
- source/tests/pt/model/test_unused_params.py (1 hunks)
Files not reviewed due to no reviewable changes (1)
- deepmd/pt/infer/deep_eval.py
Additional context used
Ruff
source/tests/pt/common.py
74-74: Multiple
isinstancecalls foratom_types, merge into a single callMerge
isinstancecalls foratom_types(SIM101)
81-81: Multiple
isinstancecalls foratom_types, merge into a single callMerge
isinstancecalls foratom_types(SIM101)
Additional comments not posted (13)
source/tests/pt/model/test_trans_denoise.py (1)
17-19: LGTM!The change in the import path for the
eval_modelfunction looks good. It improves the organization and modularity of the codebase without affecting the functionality.source/tests/pt/model/test_permutation_denoise.py (1)
17-19: LGTM! The import change aligns with the PR objectives.The relocation of the
eval_modelfunction import fromdeepmd.pt.infer.deep_evalto..commonis consistent with the goal of moving the function to the tests directory and removing it from the public API. This change contributes to the overall restructuring and maintainability of the codebase without altering the functionality of the code.source/tests/pt/model/test_unused_params.py (1)
17-19: LGTM!The change in the import path for the
eval_modelfunction is consistent with the PR objective of moving it to the tests directory. As long as the function is correctly defined in the new location, this change will not affect the functionality of the test methods.source/tests/pt/model/test_rot_denoise.py (1)
17-19: LGTM!The import statement update is consistent with the relocation of
eval_modelto the tests directory. This change aligns with the PR objective and does not appear to affect the functionality of the tests.source/tests/pt/model/test_smooth_denoise.py (1)
17-19: LGTM!The import statement change is consistent with the PR objective of moving the
eval_modelfunction to the tests directory. The relative import from..commoncorrectly points to the new location of the function.source/tests/pt/model/test_trans.py (1)
17-19: LGTM!The change in import statement for the
eval_modelfunction is consistent with the relocation of the function fromdeepmd.pt.infer.deep_evalto..commonwithin the tests directory. This aligns with the PR objective of moving the function out of the public API and into the tests.The relative import from
..commonsuggests that the function is now located in a shared location accessible by multiple test files, which promotes code reuse and maintainability within the tests directory.source/tests/pt/model/test_null_input.py (1)
22-24: LGTM!The import change for the
eval_modelfunction fromdeepmd.pt.infer.deep_evalto..commonis consistent with the AI-generated summary and does not introduce any issues. The restructuring of the import path suggests that the function has been moved to a different module without altering its functionality or usage within the code.source/tests/pt/model/test_forward_lower.py (1)
20-22: LGTM!The import location change for the
eval_modelfunction looks good. Consolidating it into thecommonmodule improves code organization and maintainability.source/tests/pt/model/test_rot.py (1)
17-19: Verify all usages ofeval_modelhave been updated.Moving
eval_modelto a common module is a good refactoring decision to improve code organization and reusability.Please ensure that all usages of
eval_modelhave been updated to import from the new location (..common). You can use the following script to verify this:Verification successful
Approve the import change, but flag remaining old imports.
The import change in
source/tests/pt/model/test_rot.pyis correct and consistent with other test files. Theeval_modelfunction is now being imported from..common, which aligns with the refactoring decision to improve code organization.However, there are still some files using the old import statement
from deepmd.pt.infer.deep_eval import. These files should be updated to use the new import location:
- ./deepmd/backend/pytorch.py
- ./source/tests/pt/model/test_deeppot.py
- ./source/tests/pt/test_init_frz_model.py
- ./source/tests/pt/test_init_model.py
Please ensure these remaining files are updated to use the new import statement for consistency across the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `eval_model` import from the new location. # Test: Search for usages still importing from the old location. Expect: No results. rg --type python -w $'from deepmd.pt.infer.deep_eval import eval_model' # Test: Search for usages importing from the new location. Expect: All relevant files. rg --type python -w $'from ..common import eval_model'Length of output: 190
Script:
#!/bin/bash # Description: Verify all usages of `eval_model` import using grep # Search for usages still importing from the old location echo "Searching for old import statements:" grep -rn "from deepmd.pt.infer.deep_eval import" . --include="*.py" -A 2 echo -e "\nSearching for new import statements:" # Search for usages importing from the new location grep -rn "from ..common import" . --include="*.py" -A 2 echo -e "\nSearching for eval_model usage:" # Search for eval_model usage to catch any missed imports grep -rn "eval_model" . --include="*.py" -A 2Length of output: 19080
source/tests/pt/model/test_autodiff.py (2)
Line range hint
27-34: LGTM!The addition of the model-related imports enhances the testing coverage by incorporating different model configurations. This change does not introduce any compatibility issues and is a positive step towards comprehensive testing.
24-26: Verify the usage ofeval_modelacross the codebase.The change in the import path for
eval_modelaligns with the PR objective of relocating the function to the tests directory. Please ensure that all other occurrences ofeval_modelin the codebase have been updated to use the new import path.Run the following script to verify the usage of
eval_model:Verification successful
Import change for
eval_modelis correctly implemented and consistent.The
eval_modelfunction is now properly imported from the..commonmodule in the filesource/tests/pt/model/test_autodiff.py. This change is consistent with other test files in the same directory. The relocation ofeval_modelto the tests directory has been successfully implemented, and there are no remaining old import paths found in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all imports of `eval_model` match the new path. # Test: Search for the function import. Expect: Only occurrences of the new import path. rg --type python -w $'from \.\.(common|tests\.common) import eval_model'Length of output: 104
Script:
#!/bin/bash # Description: Verify all imports of `eval_model` in Python files # Find all Python files echo "Python files in the repository:" fd '\.py$' echo "\nImports of eval_model:" # Search for any import of eval_model rg 'from .* import .*eval_model' --type py echo "\nSpecific import from ..common:" # Search for the specific new import path rg 'from \.\.(common|tests\.common) import .*eval_model' --type py echo "\nPotential old import paths:" # Search for potential old import paths (assuming it might have been in a 'utils' or 'helpers' module) rg 'from .*(utils|helpers).* import .*eval_model' --type pyLength of output: 25334
Script:
#!/bin/bash # Description: Search for any occurrence of eval_model and check the reviewed file echo "Searching for any occurrence of eval_model:" rg "eval_model" --type py echo "\nChecking the content of the file under review:" cat source/tests/pt/model/test_autodiff.pyLength of output: 11841
source/tests/pt/model/test_smooth.py (1)
17-19: LGTM!The change is consistent with the refactoring effort to move the
eval_modelfunction to the tests directory. It improves the structure and maintainability of the codebase without affecting the functionality of the code.source/tests/pt/model/test_permutation.py (1)
18-20: LGTM!The updated import statements for
eval_modelalign with the relocation of the function tosource/tests/pt/common.py. This change is consistent with the PR objective.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4153 +/- ##
==========================================
+ Coverage 83.37% 83.41% +0.04%
==========================================
Files 532 532
Lines 52166 52044 -122
Branches 3046 3046
==========================================
- Hits 43493 43413 -80
+ Misses 7726 7684 -42
Partials 947 947 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Han Wang <92130845+wanghan-iapcm@users.noreply.github.com> Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Per discussion in #4142 (comment). It should not be a public API as it lacks maintainance.
Summary by CodeRabbit
New Features
eval_modelfunction in the testing module to enhance model evaluation capabilities with various input configurations.Bug Fixes
eval_modelfunction from the main module to streamline functionality and improve code organization.Refactor
eval_modelto a common module across multiple test files for better organization and reduced dependencies.