Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .ci_support/environment-lammps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ dependencies:
- iprpy-data =2023.07.25
- dynaphopy =1.18.0
- lammpsparser =0.0.1
- pyscal3 = 3.3.1
Copy link
Copy Markdown
Contributor

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:

What is the latest version of pyscal3 Python package?

💡 Result:

The latest released version of the pyscal3 Python package on PyPI is 3.3.2 (released March 2, 2026). [1]

Source: [1]


🏁 Script executed:

cat -n .ci_support/environment-lammps.yml

Repository: pyiron/atomistics

Length of output: 344


Remove extra space in dependency specification for formatting consistency.

The pyscal3 dependency uses pyscal3 = 3.3.1 with a space after the = sign, while all other dependencies in this file use the format name =version with no space after the equals sign. Standardize this for consistency.

🔧 Proposed fix
-- pyscal3 = 3.3.1
+- pyscal3 =3.3.1
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.ci_support/environment-lammps.yml at line 11, Summary: The dependency
specification for pyscal3 has an extra space after the equals sign causing
inconsistent formatting. Fix: update the dependency line containing "pyscal3 =
3.3.1" to remove the space so it matches the project's style (i.e., use "pyscal3
=3.3.1"), ensuring consistency with other lines in the file.

2 changes: 1 addition & 1 deletion src/atomistics/calculators/lammps/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
LAMMPS_TIMESTEP = "timestep {{timestep}}"


LAMMPS_VELOCITY = "velocity all create $(2 * {{ temp }}) {{seed}} dist {{dist}}"
LAMMPS_VELOCITY = "velocity all create $({{velocity_rescale_factor}} * {{ temp }}) {{seed}} dist {{dist}}"


LAMMPS_ENSEMBLE_NPT = "fix ensemble all npt temp {{Tstart}} {{Tstop}} {{Tdamp}} iso {{Pstart}} {{Pstop}} {{Pdamp}}"
Expand Down
2 changes: 2 additions & 0 deletions src/atomistics/calculators/lammps/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def lammps_thermal_expansion_loop(
Pdamp: float = 1.0,
seed: int = 4928459,
dist: str = "gaussian",
velocity_rescale_factor: float = 2.0,
lmp=None,
output_keys=OutputThermalExpansion.keys(),
**kwargs,
Expand All @@ -115,6 +116,7 @@ def lammps_thermal_expansion_loop(
timestep=timestep,
seed=seed,
dist=dist,
velocity_rescale_factor=velocity_rescale_factor,
),
lmp=lmp,
**kwargs,
Expand Down
51 changes: 38 additions & 13 deletions src/atomistics/calculators/lammps/libcalculator.py
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)
+ "\n"
+ LAMMPS_THERMO_STYLE
+ "\n"
Expand Down Expand Up @@ -154,12 +155,12 @@ def calc_molecular_dynamics_nvt_with_lammpslib(
timestep: float = 0.001,
seed: int = 4928459,
dist: str = "gaussian",
disable_initial_velocity: bool = False,
velocity_rescale_factor: Optional[float] = 2.0,
lmp=None,
output_keys=OutputMolecularDynamics.keys(),
**kwargs,
) -> dict:
if not disable_initial_velocity:
if velocity_rescale_factor is not None:
init_str = (
LAMMPS_THERMO_STYLE
+ "\n"
Expand All @@ -180,6 +181,7 @@ def calc_molecular_dynamics_nvt_with_lammpslib(
timestep=timestep,
seed=seed,
dist=dist,
velocity_rescale_factor=velocity_rescale_factor,
)
else:
init_str = (
Expand Down Expand Up @@ -232,12 +234,17 @@ def calc_molecular_dynamics_npt_with_lammpslib(
Pdamp: float = 1.0,
seed: int = 4928459,
dist: str = "gaussian",
disable_initial_velocity: bool = False,
velocity_rescale_factor: Optional[float] = 2.0,
couple_xyz: bool = False,
lmp=None,
output_keys=OutputMolecularDynamics.keys(),
**kwargs,
) -> dict:
if not disable_initial_velocity:
if couple_xyz:
LAMMPS_ENSEMBLE_NPT_XYZ = LAMMPS_ENSEMBLE_NPT + " couple xyz"
else:
LAMMPS_ENSEMBLE_NPT_XYZ = LAMMPS_ENSEMBLE_NPT
if velocity_rescale_factor is not None:
init_str = (
LAMMPS_THERMO_STYLE
+ "\n"
Expand All @@ -247,7 +254,7 @@ def calc_molecular_dynamics_npt_with_lammpslib(
+ "\n"
+ LAMMPS_VELOCITY
+ "\n"
+ LAMMPS_ENSEMBLE_NPT
+ LAMMPS_ENSEMBLE_NPT_XYZ
)
input_template = Template(init_str).render(
thermo=thermo,
Expand All @@ -261,6 +268,7 @@ def calc_molecular_dynamics_npt_with_lammpslib(
timestep=timestep,
seed=seed,
dist=dist,
velocity_rescale_factor=velocity_rescale_factor,
)
else:
init_str = (
Expand All @@ -270,7 +278,7 @@ def calc_molecular_dynamics_npt_with_lammpslib(
+ "\n"
+ LAMMPS_THERMO
+ "\n"
+ LAMMPS_ENSEMBLE_NPT
+ LAMMPS_ENSEMBLE_NPT_XYZ
)
input_template = Template(init_str).render(
thermo=thermo,
Expand Down Expand Up @@ -314,12 +322,12 @@ def calc_molecular_dynamics_nph_with_lammpslib(
Pdamp: float = 1.0,
seed: int = 4928459,
dist: str = "gaussian",
disable_initial_velocity: bool = False,
velocity_rescale_factor: Optional[float] = 2.0,
lmp=None,
output_keys=OutputMolecularDynamics.keys(),
**kwargs,
) -> dict:
if not disable_initial_velocity:
if velocity_rescale_factor is not None:
init_str = (
LAMMPS_THERMO_STYLE
+ "\n"
Expand All @@ -340,6 +348,7 @@ def calc_molecular_dynamics_nph_with_lammpslib(
timestep=timestep,
seed=seed,
dist=dist,
velocity_rescale_factor=velocity_rescale_factor,
)
else:
init_str = (
Expand Down Expand Up @@ -389,12 +398,12 @@ def calc_molecular_dynamics_langevin_with_lammpslib(
Tdamp: float = 0.1,
seed: int = 4928459,
dist: str = "gaussian",
disable_initial_velocity: bool = False,
velocity_rescale_factor: Optional[float] = 2.0,
lmp=None,
output_keys=OutputMolecularDynamics.keys(),
**kwargs,
):
if not disable_initial_velocity:
if velocity_rescale_factor is not None:
init_str = (
LAMMPS_THERMO_STYLE
+ "\n"
Expand All @@ -417,6 +426,7 @@ def calc_molecular_dynamics_langevin_with_lammpslib(
timestep=timestep,
seed=seed,
dist=dist,
velocity_rescale_factor=velocity_rescale_factor,
)
else:
init_str = (
Expand Down Expand Up @@ -473,6 +483,7 @@ def calc_molecular_dynamics_thermal_expansion_with_lammpslib(
Pdamp: float = 1.0,
seed: int = 4928459,
dist: str = "gaussian",
couple_xyz: bool = False,
lmp=None,
output_keys=OutputThermalExpansion.keys(),
**kwargs,
Expand All @@ -487,7 +498,11 @@ def calc_molecular_dynamics_thermal_expansion_with_lammpslib(
+ LAMMPS_VELOCITY
+ "\n"
)
run_str = LAMMPS_ENSEMBLE_NPT + "\n" + LAMMPS_RUN
if couple_xyz:
LAMMPS_ENSEMBLE_NPT_XYZ = LAMMPS_ENSEMBLE_NPT + " couple xyz"
else:
LAMMPS_ENSEMBLE_NPT_XYZ = LAMMPS_ENSEMBLE_NPT
run_str = LAMMPS_ENSEMBLE_NPT_XYZ + "\n" + LAMMPS_RUN
temperature_lst = np.arange(Tstart, Tstop + Tstep, Tstep).tolist()
return lammps_thermal_expansion_loop(
structure=structure,
Expand Down Expand Up @@ -593,3 +608,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 +613 to +620
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Type check is too restrictive - rejects valid integer inputs.

The isinstance(vmax, float) check on line 615 will reject integer values like vmax=1, which are valid numeric inputs. In Python, integers are commonly used interchangeably with floats for numeric parameters.

🐛 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.")
+            raise TypeError("vmax must be a numeric value (int or float).")
     else:
         return LAMMPS_MINIMIZE_VOLUME
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/atomistics/calculators/lammps/libcalculator.py` around lines 613 - 620,
The type check in _get_vmax_command currently only accepts float and will
wrongly reject integer inputs; change the check to accept both ints and floats
(e.g., use isinstance(vmax, (int, float)) or numbers.Real), convert vmax to
float when formatting into the command, and update the TypeError message to
reflect "vmax must be a number" while keeping the fallback to return
LAMMPS_MINIMIZE_VOLUME unchanged; refer to function _get_vmax_command and
constant LAMMPS_MINIMIZE_VOLUME to locate and modify the logic.

Loading
Loading