chore(pt): make comm_dict for dpa2 noncompulsory when nghost is 0
#4144
Conversation
Signed-off-by: Jinzhe Zeng <[email protected]>
Signed-off-by: Jinzhe Zeng <[email protected]>
WalkthroughWalkthroughThe changes involve modifications to the Changes
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: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- source/api_cc/src/DeepPotPT.cc (3 hunks)
- source/api_cc/tests/test_deeppot_a_fparam_aparam_pt.cc (1 hunks)
- source/api_cc/tests/test_deeppot_dpa_pt.cc (3 hunks)
- source/api_cc/tests/test_deeppot_pt.cc (1 hunks)
Additional comments not posted (7)
source/api_cc/tests/test_deeppot_a_fparam_aparam_pt.cc (1)
98-98: Possible residual file due to emptyTearDown()methodThe
TearDown()method is now empty after removing the file deletion operation. Iffparam_aparam.pbis still being generated during tests, it may remain on the filesystem after tests complete, potentially affecting subsequent test runs or causing unintended side effects.Please verify whether
fparam_aparam.pbis still created during the tests. If it is no longer generated or needed, consider removing the emptyTearDown()method to clean up the code. If the file is still generated, reintroducing the file deletion ensures a clean test environment.Run the following script to check if
fparam_aparam.pbis being generated or used:Verification successful
Remove empty
TearDown()methodThe empty
TearDown()method insource/api_cc/tests/test_deeppot_a_fparam_aparam_pt.cccan be safely removed. Our verification confirms that:
- The file does not create, use, or manipulate 'fparam_aparam.pb'.
- There are no visible file operations or resource allocations that would require cleanup.
Removing this empty method will improve code cleanliness without affecting the test's functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for code that creates or manipulates 'fparam_aparam.pb' # Test: Find any occurrences of 'fparam_aparam.pb' in the codebase # Expectation: No code should be generating or using 'fparam_aparam.pb' rg 'fparam_aparam\.pb' --type cppLength of output: 5977
Script:
#!/bin/bash # Description: Search for any usage of 'fparam_aparam.pb' in the file under review and check for other potential cleanup operations # Search for 'fparam_aparam.pb' in the specific file echo "Searching for 'fparam_aparam.pb' usage in test_deeppot_a_fparam_aparam_pt.cc:" rg 'fparam_aparam\.pb' source/api_cc/tests/test_deeppot_a_fparam_aparam_pt.cc # Check for any file operations or resource allocations that might need cleanup echo -e "\nChecking for file operations or resource allocations:" rg -i '(fopen|malloc|new|create)' source/api_cc/tests/test_deeppot_a_fparam_aparam_pt.ccLength of output: 464
source/api_cc/tests/test_deeppot_dpa_pt.cc (4)
39-39: Corrected the order of return values in the commentThe updated comment now correctly reflects the order of the returned values from
dp.evalase, f, v, ae, av. This improves code clarity.
227-227: Verify the model path indp.initThe model is initialized using a relative path
"../../tests/infer/deeppot_dpa.pth". Ensure that this path is correct and accessible during test execution to prevent file not found errors.
174-174: Handling emptyboxfor non-periodic boundary conditionsInitializing
boxas an empty vector correctly represents non-periodic boundary conditions. Confirm that theDeepPotmodel anddp.computemethod properly handle an emptyboxwithout errors.
250-393: Addition of comprehensive tests for non-periodic boundary conditionsThe new test cases in
TestInferDeepPotDpaPtNopbceffectively validate theDeepPotmodel under non-periodic boundary conditions. The use of atomic computations and neighbor lists enhances test coverage.source/api_cc/src/DeepPotPT.cc (2)
167-167: Declaration ofmapping_tensorThe declaration of
c10::optional<torch::Tensor> mapping_tensor;is appropriate to allow for an optional tensor that may or may not be initialized based on conditions.
200-205: Proper Initialization ofmapping_tensorWhennghost == 0The code correctly initializes
mapping_tensorwhendo_message_passing == 1 && nghost == 0, handling the case where there are no ghost atoms (e.g., serial non-periodic boundary conditions). This ensures thatmapping_tensoris set to a valid tensor for subsequent processing.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #4144 +/- ##
==========================================
+ Coverage 83.09% 83.12% +0.02%
==========================================
Files 533 533
Lines 52221 52313 +92
Branches 3032 3046 +14
==========================================
+ Hits 43394 43484 +90
+ Misses 7881 7879 -2
- Partials 946 950 +4 ☔ View full report in Codecov by Sentry. |
enhancement on #4144 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced tensor mapping capabilities with the addition of a new `mapping_tensor` variable. - Updated `compute` method to handle ghost atoms and support improved tensor creation logic. - Overloaded `computew` methods to support both double and float types. - **Bug Fixes** - Improved error handling in the `translate_error` method for better exception management. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…4237) fix errors mentioned in following pr: #4220 #4209 #4144 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced message passing logic in the computation process for improved efficiency. - Added new test functions to evaluate DeepMD model performance under various conditions. - **Bug Fixes** - Improved error handling and assertions in test cases to ensure robustness. - **Refactor** - Streamlined tensor operations in the communication process to enhance clarity and reduce unnecessary computations. - Removed outdated test cases related to neighbor list handling in the DeepPot class. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Construct and pass a mapping instead when there is no ghost atom.
Summary by CodeRabbit
New Features
DeepPotmodel without periodic boundary conditions, enhancing testing coverage.Bug Fixes
Documentation
Chores
TearDownmethods in multiple test classes, removing unnecessary file deletion steps.