-
Notifications
You must be signed in to change notification settings - Fork 516
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
doc: update spin lmp doc and example #4375
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant updates to the documentation and configuration files related to spin settings and their integration into energy models for TensorFlow and PyTorch, as well as enhancements for LAMMPS. Key changes include the addition of sections on "Spin Loss" and "Spin data format" in the TensorFlow and PyTorch documentation, and the introduction of a new pair style Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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: 1
🧹 Outside diff range and nitpick comments (5)
examples/spin/lmp/in.force (1)
Line range hint
44-49
: Document minimization alternativesConsider adding comments to explain:
- When to use the default CG minimization
- When the spin/lbfgs alternative might be preferred
min_style cg min_modify line backtrack minimize 1.0e-10 1.0e-4 5000 10000 +# Alternative minimization for spin systems: +# Use spin/lbfgs when dealing with complex magnetic configurations #min_style spin/lbfgs #min_modify line spin_cubic discrete_factor 10.0 #minimize 1.0e-10 1.0e-10 1000 5000doc/model/train-energy-spin.md (1)
85-85
: Fix variable naming inconsistency in the prefactor explanation.The variable naming is inconsistent between the LaTeX formula and the text explanation:
- LaTeX formula uses
$p_{fr}$ - Text example refers to
$p_f$ Standardize the variable naming to avoid confusion.
doc/third-party/lammps-command.md (3)
105-105
: Consider adding more details about magnetic force predictionsWhile the documentation now mentions both force and magnetic force predictions for spin models, it would be helpful to include:
- The units of magnetic force predictions
- How these predictions differ from regular force predictions
- Any specific considerations when interpreting these values
72-73
: Consider explaining virial limitation for spin modelsWhile the documentation notes that virial is not supported in spin models, it would be helpful to:
- Explain why this limitation exists
- Mention if this is a temporary limitation
- Provide alternative approaches if available
95-96
: Consider enhancing spin examples with commentsThe examples demonstrate the basic usage well, but could be more educational with:
- Comments explaining the expected behavior
- Typical use cases for single vs. multiple spin models
- Example output format for spin calculations
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
doc/model/train-energy-spin.md
(1 hunks)doc/third-party/lammps-command.md
(4 hunks)examples/spin/lmp/in.force
(1 hunks)
🔇 Additional comments (5)
examples/spin/lmp/in.force (3)
Line range hint 32-32
: LGTM: Enhanced output with magnetic energy
The addition of v_emag
to thermo_style provides valuable magnetic energy tracking.
Line range hint 35-37
: LGTM: Enhanced dump configuration
Good improvements:
- Added spin components for more detailed analysis
- Increased floating-point precision for better accuracy
9-9
: Verify impact of removed parameters
The removal of virtual_len
and spin_norm
parameters from the pair style configuration might affect existing simulations. Please confirm:
- Whether these parameters have default values in the new
deepspin
style - If existing simulations need to be updated
✅ Verification successful
The previous results show these parameters are part of the model training configuration. Let's verify how they are handled in the LAMMPS implementation and if there are any other examples showing the correct usage of the deepspin pair style.
Parameters are handled by the model, not the pair style command
Based on the documentation, virtual_len
and spin_norm
are model training parameters that are embedded in the trained neural network model (.pb
file), not runtime parameters for the LAMMPS pair style command. The change from deepmd
to deepspin
is correct, and the simplified syntax without these parameters is the proper usage as shown in the documentation examples.
Key findings:
deepspin
is specifically designed for spin simulations- Parameters are configured during model training (as seen in
doc/model/train-energy-spin.md
) - The pair style command only needs the model file path as shown in the documentation example:
pair_style deepspin spin.pb
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other simulation files using these parameters
rg -l "virtual_len|spin_norm" --type-add 'lmp:*.lmp' -t lmp
# Look for documentation about these parameters
fd -e md -e rst . | xargs rg "virtual_len|spin_norm"
Length of output: 1030
Script:
#!/bin/bash
# Search for documentation about deepspin pair style
fd -e md -e rst . | xargs rg -i "deepspin|pair_style.*deepspin" -B2 -A2
# Look for example files using deepspin
rg -l "pair_style.*deepspin" --type-add 'lmp:*.lmp' -t lmp
# Check if there are any other spin-related parameter settings in example files
rg "pair_.*spin" --type-add 'lmp:*.lmp' -t lmp -A2 -B2
Length of output: 1860
doc/third-party/lammps-command.md (2)
40-45
: Well-structured introduction of pair styles!
The documentation clearly distinguishes between deepmd
and deepspin
pair styles, providing appropriate context and example references for each use case.
103-103
: Comprehensive update of supported model formats!
The documentation now correctly reflects all supported model file formats and backend options.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4375 +/- ##
==========================================
+ Coverage 84.50% 84.54% +0.03%
==========================================
Files 596 597 +1
Lines 56665 56819 +154
Branches 3459 3487 +28
==========================================
+ Hits 47884 48036 +152
- Misses 7654 7656 +2
Partials 1127 1127 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
the deepmd
and deepspin
are two different pair styles, I strongly recommend provide separate doc sections for the them, which follows the lammps convention.
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
🧹 Outside diff range and nitpick comments (1)
doc/third-party/lammps-command.md (1)
128-202
: Well-structured documentation for the newdeepspin
pair style.The documentation is comprehensive and follows a consistent format with the
deepmd
section. The examples effectively demonstrate various usage scenarios.Consider removing the duplicate note about virial limitations. The note appears twice (lines 143-145 and 195-197). Keeping it only at the beginning of the section would be sufficient.
- :::{note} - Please note that the virial and atomic virial are not currently supported in spin models. - :::
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/third-party/lammps-command.md
(4 hunks)
🔇 Additional comments (2)
doc/third-party/lammps-command.md (2)
42-43
: LGTM! Clear and accurate documentation updates.
The changes effectively clarify the purpose of the deepmd
pair style and provide accurate technical details about supported model file formats. The cross-reference to the new deepspin
section helps users navigate to the appropriate documentation for spin-based simulations.
Also applies to: 97-97, 122-122
310-310
: LGTM! Proper citation added.
The DPSPIN citation is correctly formatted and provides the appropriate reference for the spin-based functionality.
Co-authored-by: Jinzhe Zeng <[email protected]> Signed-off-by: Duo <[email protected]>
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: 1
🧹 Outside diff range and nitpick comments (1)
doc/third-party/lammps-command.md (1)
143-145
: Remove duplicate note about virial limitations.The note about virial limitations appears twice in the documentation. Consider keeping only one instance, preferably at the beginning of the section.
-:::{note} -Please note that the virial and atomic virial are not currently supported in spin models. -:::Also applies to: 195-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/third-party/lammps-command.md
(4 hunks)
🔇 Additional comments (2)
doc/third-party/lammps-command.md (2)
42-43
: LGTM! Clear distinction between traditional and spin simulations.
The documentation clearly differentiates between the use cases of deepmd
and deepspin
pair styles, helping users choose the appropriate style for their simulations.
Also applies to: 122-122
310-310
: LGTM! Well-documented reference.
The addition of the DeepSPIN paper reference with DOI provides proper attribution and allows users to find more detailed information about the methodology.
Co-authored-by: Han Wang <[email protected]> Signed-off-by: Duo <[email protected]>
Co-authored-by: Han Wang <[email protected]> Signed-off-by: Duo <[email protected]>
for more information, see https://pre-commit.ci
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: 1
🧹 Outside diff range and nitpick comments (4)
doc/third-party/lammps-command.md (4)
96-96
: Consider consistent formatting for file extensionsFor better readability, consider using a consistent format for file extensions:
-This pair style takes the deep potential defined in a model file that usually has .pb/.pth/.savedmodel extensions. +This pair style takes the deep potential defined in a model file that usually has `.pb`, `.pth`, or `.savedmodel` extensions.
183-183
: Fix grammatical issue-Evaluate the interaction of the system with spin by using [DeepSPIN][DPSPIN] models. It is noticed that deep spin model is not a "pairwise" interaction, but a multi-body interaction. +Evaluate the interaction of the system with spin by using [DeepSPIN][DPSPIN] models. It is noticed that the deep spin model is not a "pairwise" interaction, but a multi-body interaction.🧰 Tools
🪛 LanguageTool
[uncategorized] ~183-~183: You might be missing the article “the” here.
Context: ...PIN][DPSPIN] models. It is noticed that deep spin model is not a "pairwise" interact...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
140-142
: Remove duplicate note about virial supportThe note about virial support is duplicated. Consider keeping only one instance, preferably at the beginning of the section.
Remove the second note at lines 192-194.
Also applies to: 192-194
167-179
: Enhance examples with spin-specific scenariosWhile the current examples are valid, consider adding more spin-specific examples to better illustrate the unique features of
deepspin
.Add examples like:
# Example with spin-specific parameters pair_style deepspin spin_model.pb fparam 3.0 1.0 # magnetic field components pair_coeff * * Fe Ni # magnetic materials # Example with multiple spin models for deviation analysis pair_style deepspin spin_1.pb spin_2.pb spin_3.pb out_file spin_dev.out atomic
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/third-party/lammps-command.md
(4 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/third-party/lammps-command.md
[uncategorized] ~183-~183: You might be missing the article “the” here.
Context: ...PIN][DPSPIN] models. It is noticed that deep spin model is not a "pairwise" interact...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (1)
doc/third-party/lammps-command.md (1)
307-307
: LGTM!
The DeepSPIN paper reference is properly formatted and includes the DOI link.
Summary by CodeRabbit
New Features
deepspin
, for systems that include spin in LAMMPS.Documentation