-
Notifications
You must be signed in to change notification settings - Fork 584
docs(lmp): fix the usage of LAMMPS pair_style hybrid/overlay
#4951
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
Conversation
As documented in https://docs.lammps.org/pair_hybrid.html, "In the pair_coeff commands, the name of a pair style must be added after the I,J type specification". Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
📝 WalkthroughWalkthroughA documentation update modifies the LAMMPS D3 dispersion example in doc/third-party/lammps-command.md. The pair_coeff placeholders are replaced with model-specific entries for deepmd and dispersion/d3 within a pair_style hybrid/overlay setup. No code or API changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doc/third-party/lammps-command.md (1)
313-321: Version note is too lax for the ‘original’ damping keyword.The example uses
dispersion/d3 original ..., but “original” replaced the former “zero” keyword on 2Apr2025. If a user runs exactly 4Feb2025, this example will fail unless they usezero. Please either raise the minimum version to 2Apr2025 or add a parenthetical for 4Feb2025 users.Suggested edit:
- Requires LAMMPS version 4Feb2025 or newer. + Requires LAMMPS version 2Apr2025 or newer. + (For LAMMPS 4Feb2025 specifically, use `zero` instead of `original`.)Refs: “Added in version 4Feb2025” and “Changed in version 2Apr2025: renamed zero keyword to original”. (docs.lammps.org)
🧹 Nitpick comments (1)
doc/third-party/lammps-command.md (1)
322-324: Optional: remind users about units for dispersion/d3.LAMMPS’ dispersion/d3 parameters require metal units; a one-line note under this example would prevent confusion if readers chose a different unit style earlier.
Source: “The compiled in parameters require the use of metal units.” (docs.lammps.org)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
doc/third-party/lammps-command.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Build wheels for cp311-macosx_arm64
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build C++ (cuda, cuda)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Build C library (2.14, >=2.5.0,<2.15, libdeepmd_c_cu11.tar.gz)
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Test Python (5, 3.12)
- GitHub Check: Test Python (3, 3.12)
- GitHub Check: Test Python (3, 3.9)
- GitHub Check: Test Python (2, 3.12)
- GitHub Check: Test Python (4, 3.12)
- GitHub Check: Test Python (6, 3.12)
- GitHub Check: Test Python (6, 3.9)
- GitHub Check: Test Python (5, 3.9)
- GitHub Check: Test Python (1, 3.9)
- GitHub Check: Test Python (4, 3.9)
- GitHub Check: Test Python (1, 3.12)
- GitHub Check: Test Python (2, 3.9)
- GitHub Check: Test C++ (true)
- GitHub Check: Test C++ (false)
🔇 Additional comments (1)
doc/third-party/lammps-command.md (1)
322-324: Correct hybrid/overlay pair_coeff syntax — looks good.Including the sub-style name after the I,J types is required for hybrid/overlay; your example now follows the LAMMPS rule.
See: “In the pair_coeff commands, the name of a pair style must be added after the I,J type specification…”. (docs.lammps.org)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the documentation for LAMMPS pair_style hybrid/overlay usage by correcting the pair_coeff command syntax. The fix ensures that the pair style name is properly specified after the atom type specification, as required by LAMMPS documentation.
- Corrects
pair_coeffcommands to include the pair style names (deepmdanddispersion/d3) - Aligns the documentation with official LAMMPS requirements for hybrid pair styles
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #4951 +/- ##
=======================================
Coverage 84.28% 84.28%
=======================================
Files 705 705
Lines 69085 69084 -1
Branches 3573 3572 -1
=======================================
+ Hits 58229 58230 +1
+ Misses 9715 9714 -1
+ Partials 1141 1140 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…odeling#4951) As documented in https://docs.lammps.org/pair_hybrid.html, "In the pair_coeff commands, the name of a pair style must be added after the I,J type specification". <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Documentation - Updated the hybrid/overlay example for D3 dispersion to use model-specific pair_coeff entries instead of placeholders. - Clarifies how to declare coefficients for multiple models in one configuration (e.g., Deep Potential and D3), improving accuracy and reducing setup confusion. - Example now better reflects real-world usage without altering existing commands. - No functional changes to the software; updates are limited to documentation examples. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
As documented in https://docs.lammps.org/pair_hybrid.html, "In the pair_coeff commands, the name of a pair style must be added after the I,J type specification".
Summary by CodeRabbit