Skip to content

LAMMPS: Add vmax option for structure optimization #607

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.

Sign up for GitHub

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

Open
jan-janssen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

LAMMPS: Add vmax option for structure optimization #607

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/atomistics/calculators/lammps/libcalculator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Optional

import numpy as np
import pandas
Expand Down Expand Up @@ -49,11 +49,12 @@ def optimize_positions_and_volume_with_lammpslib(
maxiter: int = 100000,
maxeval: int = 10000000,
thermo: int = 10,
vmax: Optional[float] = None,
lmp=None,
**kwargs,
) -> Atoms:
template_str = (
LAMMPS_MINIMIZE_VOLUME
_get_vmax_command(vmax=vmax)
Comment on lines +52 to +57
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

LAMMPS fix box/relax vmax parameter documentation valid values

💡 Result:

The vmax value is a positive fractional limit on how much the box may change in one minimizer iteration. Valid values:

  • Any number > 0.0 (should be much less than 1.0).
  • Interpreted as maximum fractional volume change per iteration when using couple xyz; otherwise it limits fractional change of any linear box dimension.
  • Example recommended value: 0.001.
  • Default if not specified: 0.0001.

References: LAMMPS fix box/relax documentation. [1][2]

Sources
[1] LAMMPS fix box/relax manual (release).
[2] LAMMPS documentation / examples (fix box/relax description).

🏁 Script executed:

# Find the _get_vmax_command function to understand implementation
fd -t f "libcalculator.py" | head -20

Repository: pyiron/atomistics

Length of output: 111

🏁 Script executed:

# Search for vmax-related code and usage
rg "vmax" -B 3 -A 5 "src/atomistics/calculators/lammps/libcalculator.py" | head -60

Repository: pyiron/atomistics

Length of output: 704

🏁 Script executed:

# Find the _get_vmax_command function definition
rg "_get_vmax_command" -B 2 -A 10 --type py

Repository: pyiron/atomistics

Length of output: 1854

Add validation for vmax parameter to ensure it is positive.

According to LAMMPS documentation, vmax must be a positive number (> 0.0). The current implementation only checks if the value is a float type, but does not validate the valid range. This should be enforced in _get_vmax_command() to provide immediate feedback to users rather than letting LAMMPS fail at runtime.

Suggested validation addition 
def _get_vmax_command(vmax: Optional[float]) -> str:
    if vmax is not None:
        if isinstance(vmax, float):
            if vmax <= 0.0:
                raise ValueError(f"vmax must be positive (> 0.0), got {vmax}")
            return LAMMPS_MINIMIZE_VOLUME + " vmax {vmax}".format(vmax=vmax)
        else:
            raise TypeError("vmax must be a float.")
    else:
        return LAMMPS_MINIMIZE_VOLUME
🤖 Prompt for AI Agents 
In @src/atomistics/calculators/lammps/libcalculator.py around lines 52 - 57, The
_get_vmax_command function currently only checks type but not range; update it
to validate that if vmax is not None it must be a float and strictly > 0.0,
otherwise raise ValueError (for non-positive values) or TypeError (for wrong
type); when valid, return the existing command string with the vmax inserted
(e.g., LAMMPS_MINIMIZE_VOLUME + " vmax {vmax}"), otherwise return
LAMMPS_MINIMIZE_VOLUME when vmax is None.

+ "\n"
+ LAMMPS_THERMO_STYLE
+ "\n"
Expand Down Expand Up @@ -593,3 +594,13 @@ def evaluate_with_lammpslib(
)
lmp.close()
return results_dict


def _get_vmax_command(vmax: Optional[float]) -> str:
if vmax is not None:
if isinstance(vmax, float):
return LAMMPS_MINIMIZE_VOLUME + " vmax {vmax}".format(vmax=vmax)
else:
raise TypeError("vmax must be a float.")
else:
return LAMMPS_MINIMIZE_VOLUME
Comment on lines +599 to +606
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix type check to accept integers.

The isinstance(vmax, float) check on Line 601 will reject integer values (e.g., vmax=5), even though integers are valid numeric inputs for this parameter. In Python, isinstance(5, float) returns False.

🐛 Proposed fix to accept both int and float 
 def _get_vmax_command(vmax: Optional[float]) -> str:
     if vmax is not None:
-        if isinstance(vmax, float):
+        if isinstance(vmax, (int, float)):
             return LAMMPS_MINIMIZE_VOLUME + " vmax {vmax}".format(vmax=vmax)
         else:
             raise TypeError("vmax must be a float.")
     else:
         return LAMMPS_MINIMIZE_VOLUME

Optional: Consider refactoring the error message.

As per static analysis hint (TRY003), consider using a shorter error message or defining a custom exception class for consistency with Python conventions.

♻️ Example refactor 
 def _get_vmax_command(vmax: Optional[float]) -> str:
     if vmax is not None:
         if isinstance(vmax, (int, float)):
             return LAMMPS_MINIMIZE_VOLUME + " vmax {vmax}".format(vmax=vmax)
         else:
-            raise TypeError("vmax must be a float.")
+            raise TypeError(f"Expected numeric vmax, got {type(vmax).__name__}")
     else:
         return LAMMPS_MINIMIZE_VOLUME
🧰 Tools
🪛 Ruff (0.14.10)

604-604: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents 
In @src/atomistics/calculators/lammps/libcalculator.py around lines 599 - 606,
The _get_vmax_command function currently uses isinstance(vmax, float) which
rejects integer inputs like vmax=5; update the type check in _get_vmax_command
to accept both int and float (e.g., isinstance(vmax, (int, float)) or use
numbers.Real) and format the value into the returned string unchanged; also
shorten the TypeError message (e.g., "vmax must be a number") for consistency or
replace with a custom exception if your codebase prefers.

Loading