merge devel to master to release v0.2.24 #808
Conversation
<!--pre-commit.ci start--> updates: - [github.com/astral-sh/ruff-pre-commit: v0.9.6 → v0.9.7](astral-sh/ruff-pre-commit@v0.9.6...v0.9.7) <!--pre-commit.ci end--> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
<!--pre-commit.ci start--> updates: - [github.com/astral-sh/ruff-pre-commit: v0.9.7 → v0.9.9](astral-sh/ruff-pre-commit@v0.9.7...v0.9.9) <!--pre-commit.ci end--> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ed (deepmodeling#800) The old version will induce a bug that dpdata will always store the spin information, and when dpdata save it to lammps format, the spin information will also be written, which is invalid for a non-spin lammps job. Now, the spin information is saved to dpdata only when the magnetic moment of one atom is specified, which is necessary for a spin job. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Spin data is now processed to include only explicitly defined moments, ensuring accurate output. - **Documentation** - User-facing documentation has been updated to clarify when spin information appears. - **Tests** - Test entries for atomic positions have been streamlined by removing redundant markers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: root <pxlxingliang> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
<!--pre-commit.ci start--> updates: - [github.com/astral-sh/ruff-pre-commit: v0.9.9 → v0.9.10](astral-sh/ruff-pre-commit@v0.9.9...v0.9.10) <!--pre-commit.ci end--> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Hi,
I think this part of the code was attempting to modify local variables
(`ii`) rather than the actual array elements in `self.data["coords"]`.
Before:
```
@post_funcs.register("shift_orig_zero")
def _shift_orig_zero(self):
for ff in self.data["coords"]:
for ii in ff:
ii = ii - self.data["orig"]
...
```
After:
```
...
self.data["coords"] = self.data["coords"] - self.data["orig"]
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Refactor**
- Enhanced the efficiency of coordinate adjustments for improved
performance on larger datasets.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: .Del <10773382+zrzrv5@users.noreply.github.com>
<!--pre-commit.ci start--> updates: - [github.com/astral-sh/ruff-pre-commit: v0.9.10 → v0.11.0](astral-sh/ruff-pre-commit@v0.9.10...v0.11.0) <!--pre-commit.ci end--> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
CodSpeed Performance ReportMerging #808 will not alter performanceComparing Summary
|
📝 WalkthroughWalkthroughThis update modifies several aspects of the codebase. The Ruff pre-commit hook version in the configuration file is bumped from v0.9.6 to v0.11.0. In the abacus structure file, a boolean flag is introduced in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant parse_pos
participant get_frame_from_stru
Caller->>parse_pos: Call parse_pos with atomic positions
parse_pos->>parse_pos: Initialize define_atom_mag = False
parse_pos->>parse_pos: Iterate over positions to check for non-None imagmom
alt Magnetic moment found
parse_pos->>parse_pos: Set define_atom_mag = True and populate mags
else No magnetic moments
parse_pos->>parse_pos: Leave mags empty
end
parse_pos-->>Caller: Return parsed positions and mags
Caller->>get_frame_from_stru: Request frame construction
get_frame_from_stru->>get_frame_from_stru: Include 'spins' key only if mags is not empty
get_frame_from_stru-->>Caller: Return frame data
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 0
🧹 Nitpick comments (2)
dpdata/system.py (1)
713-713: Optimization: Replaced loops with vectorized operationGreat optimization! Using NumPy's vectorized operations to subtract the origin from coordinates is much more efficient than using nested loops, which was likely the previous implementation.
The remaining operations like setting
self.data["orig"]and the assertion check are kept intact, ensuring the method's functionality is preserved while improving performance.dpdata/abacus/stru.py (1)
417-421: Conditional handling of magnetic moment dataThis change improves the function by only returning magnetic moment data when it's explicitly defined in the input, avoiding empty or default values when not needed.
Minor style improvement opportunity:
- # here return the magnetic moment only when the atom magnetic moment is specified. - if not define_atom_mag: - mags = [] - else: - mags = np.array(mags) + # here return the magnetic moment only when the atom magnetic moment is specified. + mags = [] if not define_atom_mag else np.array(mags)🧰 Tools
🪛 Ruff (0.8.2)
417-420: Use ternary operator
mags = [] if not define_atom_mag else np.array(mags)instead ofif-else-blockReplace
if-else-block withmags = [] if not define_atom_mag else np.array(mags)(SIM108)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.pre-commit-config.yaml(1 hunks)dpdata/abacus/stru.py(5 hunks)dpdata/system.py(1 hunks)tests/abacus.scf/stru_test(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
dpdata/abacus/stru.py
417-420: Use ternary operator mags = [] if not define_atom_mag else np.array(mags) instead of if-else-block
Replace if-else-block with mags = [] if not define_atom_mag else np.array(mags)
(SIM108)
🔇 Additional comments (5)
.pre-commit-config.yaml (1)
24-24: Ruff pre-commit hook version updated to v0.11.0Upgrading the Ruff linter from v0.9.6 to v0.11.0 ensures the project benefits from the latest linting improvements and bug fixes.
dpdata/abacus/stru.py (3)
365-365: Good addition: Flag to track magnetic moment definitionsAdding the
define_atom_magflag is a good approach to track whether any magnetic moments are defined during parsing.
394-396: Improved logic for tracking defined magnetic momentsThis code properly updates the flag when actual magnetic moments are defined in the input, rather than just assuming defaults.
449-456: Improved documentation for spins behaviorThe documentation for the
spinsreturn value is much clearer now, explaining that it's only returned when magnetic moments are explicitly defined in the STRU file. This helps users understand when to expect this data in the output.tests/abacus.scf/stru_test (1)
25-32: Removed redundant magnetic moment data from test fileThis change aligns the test data with the code changes in
stru.pyby removing the unnecessary magnetic moment definitions (mag 0.000000000000 0.000000000000 0.000000000000). This is consistent with the new approach of only including magnetic moment data when it's actually defined with non-default values.
3 participants
Summary by CodeRabbit