Skip to content

fix: fix nopbc in dpdata driver #4027

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

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented Jul 27, 2024

Sometimes dpdata data may contain "nopbc": False (from ase or n2p2).

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of the "nopbc" key in the data processing logic, enhancing the accuracy of cell variable assignments.

Copy link
Contributor

coderabbitai bot commented Jul 27, 2024

Walkthrough

Walkthrough

The recent changes in the driver.py file enhance the logic of the label method by modifying how the "nopbc" key is evaluated. The condition now uses the get method instead of a simple membership check, allowing for more robust handling of scenarios where "nopbc" may be present but set to False. This adjustment broadens the interpretation of the flag, ensuring better functionality and reliability.

Changes

File Change Summary
deepmd/driver.py Modified the conditional logic in the label method to use get for evaluating "nopbc", enhancing robustness.

Sequence Diagram(s)

sequenceDiagram
    participant A as User
    participant B as driver.py
    participant C as Data Dictionary

    A->>B: Call label method
    B->>C: Check for "nopbc" key
    alt "nopbc" is absent or False
        B->>B: Assign cell variable
    else "nopbc" is True
        B->>B: Proceed with alternative logic
    end
    B->>A: Return result
Loading

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

@njzjz njzjz requested a review from wanghan-iapcm July 27, 2024 00:01
Copy link

codecov bot commented Jul 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.93%. Comparing base (7f61048) to head (db052f0).
Report is 106 commits behind head on devel.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #4027      +/-   ##
==========================================
- Coverage   82.93%   82.93%   -0.01%     
==========================================
  Files         522      522              
  Lines       51036    51036              
  Branches     3030     3028       -2     
==========================================
- Hits        42326    42325       -1     
+ Misses       7764     7763       -1     
- Partials      946      948       +2     

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

@wanghan-iapcm wanghan-iapcm added this pull request to the merge queue Jul 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 27, 2024
@njzjz njzjz added this pull request to the merge queue Jul 27, 2024
Merged via the queue into deepmodeling:devel with commit 0e0fc1a Jul 27, 2024
60 checks passed
@njzjz njzjz deleted the fix-nopbc-dpdata-driver branch July 27, 2024 04:15
mtaillefumier pushed a commit to mtaillefumier/deepmd-kit that referenced this pull request Sep 18, 2024
Sometimes dpdata data may contain `"nopbc": False` (from ase or n2p2).

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

## Summary by CodeRabbit

- **Bug Fixes**
- Improved handling of the "nopbc" key in the data processing logic,
enhancing the accuracy of cell variable assignments.


<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Jinzhe Zeng <[email protected]>
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.

2 participants