Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a2e7327
Migrate potential interface from atomistics to pyiron_lammps
jan-janssen Nov 8, 2025
5f29115
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 8, 2025
e946f96
Add potential to init
jan-janssen Nov 16, 2025
a2b6303
Add potential test
jan-janssen Nov 16, 2025
d6490b6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 17, 2025
a00cf14
Fix potential location
jan-janssen Nov 17, 2025
8e00b1d
I have added tests for error handling in the `pyiron_lammps/potential…
google-labs-jules[bot] Nov 17, 2025
6f433cd
fix tests
jan-janssen Nov 17, 2025
e117f4d
fix windows test
jan-janssen Nov 17, 2025
4c2880a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 17, 2025
af34515
add more tests
jan-janssen Nov 19, 2025
1e94a9a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 19, 2025
e028f79
add another test
jan-janssen Nov 19, 2025
600001b
Merge branch 'potential' of github.com:pyiron/pyiron_lammps into pote…
jan-janssen Nov 19, 2025
985520d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 19, 2025
925865c
clean up
jan-janssen Nov 19, 2025
6045e2d
Merge branch 'potential' of github.com:pyiron/pyiron_lammps into pote…
jan-janssen Nov 19, 2025
d55975c
remove bad test
jan-janssen Nov 19, 2025
6337fc2
fix tests
jan-janssen Nov 19, 2025
6327e73
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 19, 2025
2d7f8c0
more tests
jan-janssen Nov 19, 2025
335cb48
fixes
jan-janssen Nov 19, 2025
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
3 changes: 3 additions & 0 deletions pyiron_lammps/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pyiron_lammps._version
from pyiron_lammps.output import parse_lammps_output as parse_lammps_output_files
from pyiron_lammps.potential import get_potential_by_name, get_potential_dataframe
Comment thread
coderabbitai[bot] marked this conversation as resolved.
from pyiron_lammps.structure import write_lammps_datafile as write_lammps_structure

DUMP_COMMANDS = [
Expand All @@ -15,6 +16,8 @@

__version__ = pyiron_lammps._version.__version__
__all__ = [
"get_potential_by_name",
"get_potential_dataframe",
"parse_lammps_output_files",
"write_lammps_structure",
]
380 changes: 380 additions & 0 deletions pyiron_lammps/potential.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,380 @@
import os
from pathlib import Path
from typing import Union

import pandas
from ase.atoms import Atoms

potential_installation = """
Potential installation guide:

1. Check whether iprpy-data is installed. If not, install it using:

`conda install -c conda-forge iprpy-data`

2. Check whether the resource path is set via:

```python
import os
print(os.environ["CONDA_PREFIX"])
```

3. If the resource path is set, you can call the potential using:

```python
from atomistics.calculators import get_potential_by_name


get_potential_by_name(
potential_name=my_potential,
resource_path=os.path.join(os.environ["CONDA_PREFIX"], "share", "iprpy"),
)
```

"""
Comment on lines +8 to +34
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Update module reference in installation guide.

The installation guide references atomistics.calculators on line 25, but this PR migrates the interface to pyiron_lammps. The import statement should be updated to reflect the correct module name.

Apply this diff:

-from atomistics.calculators import get_potential_by_name
+from pyiron_lammps import get_potential_by_name
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
potential_installation = """
Potential installation guide:
1. Check whether iprpy-data is installed. If not, install it using:
`conda install -c conda-forge iprpy-data`
2. Check whether the resource path is set via:
```python
import os
print(os.environ["CONDA_PREFIX"])
```
3. If the resource path is set, you can call the potential using:
```python
from atomistics.calculators import get_potential_by_name
get_potential_by_name(
potential_name=my_potential,
resource_path=os.path.join(os.environ["CONDA_PREFIX"], "share", "iprpy"),
)
```
"""
potential_installation = """
Potential installation guide:
1. Check whether iprpy-data is installed. If not, install it using:
`conda install -c conda-forge iprpy-data`
2. Check whether the resource path is set via:
🤖 Prompt for AI Agents
In pyiron_lammps/potential.py around lines 8 to 34, the installation guide
string references the old module name "atomistics.calculators" on the import
example; update that import to the new module "pyiron_lammps" and adjust the
subsequent usage example to call get_potential_by_name from pyiron_lammps (i.e.,
replace the import line and any fully-qualified calls in the string), keeping
the rest of the textual instructions unchanged.



class PotentialAbstract:
"""
The PotentialAbstract class loads a list of available potentials and sorts them. Afterwards the potentials can be
accessed through:
PotentialAbstract.<Element>.<Element> or PotentialAbstract.find_potentials_set({<Element>, <Element>}

Args:
potential_df:
default_df:
selected_atoms:
"""

def __init__(
self,
potential_df: pandas.DataFrame,
default_df: pandas.DataFrame = None,
selected_atoms: list[str] = None,
):
self._potential_df = potential_df
self._default_df = default_df
if selected_atoms is not None:
self._selected_atoms = selected_atoms
else:
self._selected_atoms = []

def find(self, element: Union[set[str], list[str], str]) -> pandas.DataFrame:
"""
Find the potentials

Args:
element (set, str): element or set of elements for which you want the possible LAMMPS potentials

Returns:
list: of possible potentials for the element or the combination of elements

"""
if isinstance(element, list):
element = set(element)
elif isinstance(element, str):
element = {element}
elif not isinstance(element, set):
raise TypeError("Only, str, list and set supported!")
return self._potential_df[
[
bool(set(element).issubset(species))
for species in self._potential_df["Species"].values
]
]

def find_by_name(self, potential_name: str) -> pandas.DataFrame:
mask = self._potential_df["Name"] == potential_name
if not mask.any():
raise ValueError(f"Potential '{potential_name}' not found in database.")
return self._potential_df[mask]

def list(self) -> pandas.DataFrame:
"""
List the available potentials

Returns:
list: of possible potentials for the element or the combination of elements
"""
return self._potential_df

def __getattr__(self, item):
return self[item]

def __getitem__(self, item):
potential_df = self.find(element=item)
selected_atoms = self._selected_atoms + [item]
return PotentialAbstract(
potential_df=potential_df,
default_df=self._default_df,
selected_atoms=selected_atoms,
)

def __str__(self) -> str:
return str(self.list())

@staticmethod
def _get_potential_df(file_name_lst, resource_path):
"""

Args:
file_name_lst (set):
resource_path (str):

Returns:
pandas.DataFrame:
"""
for path, _folder_lst, file_lst in os.walk(resource_path):
for periodic_table_file_name in file_name_lst:
if (
periodic_table_file_name in file_lst
and periodic_table_file_name.endswith(".csv")
):
return pandas.read_csv(
os.path.join(path, periodic_table_file_name),
index_col=0,
converters={
"Species": (
lambda x: x.replace("'", "").strip("[]").split(", ")
),
"Config": (
lambda x: x.replace("'", "")
.replace("\\n", "\n")
.strip("[]")
.split(", ")
),
"Filename": (
lambda x: x.replace("'", "").strip("[]").split(", ")
),
},
)
raise ValueError(
"Was not able to locate the potential files." + potential_installation
)


class LammpsPotentialFile(PotentialAbstract):
"""
The Potential class is derived from the PotentialAbstract class, but instead of loading the potentials from a list,
the potentials are loaded from a file.

Args:
potential_df:
default_df:
selected_atoms:
"""

def __init__(
self,
potential_df=None,
default_df=None,
selected_atoms=None,
resource_path=None,
):
if potential_df is None:
potential_df = self._get_potential_df(
file_name_lst={"potentials_lammps.csv"},
resource_path=resource_path,
)
super().__init__(
potential_df=potential_df,
default_df=default_df,
selected_atoms=selected_atoms,
)
self._resource_path = resource_path

def default(self):
if self._default_df is not None:
atoms_str = "_".join(sorted(self._selected_atoms))
return self._default_df[
(self._default_df["Name"] == self._default_df.loc[atoms_str].values[0])
]
return None
Comment on lines +186 to +192
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add error handling for missing default potential.

Line 190 accesses self._default_df.loc[atoms_str] without checking if atoms_str exists in the index. This will raise a KeyError if the element combination has no default defined.

Apply this diff to add proper error handling:

     def default(self):
         if self._default_df is not None:
             atoms_str = "_".join(sorted(self._selected_atoms))
+            if atoms_str not in self._default_df.index:
+                return None
             return self._default_df[
                 (self._default_df["Name"] == self._default_df.loc[atoms_str].values[0])
             ]
         return None
🤖 Prompt for AI Agents
In pyiron_lammps/potential.py around lines 186 to 192, the code directly indexes
self._default_df.loc[atoms_str] which will raise a KeyError if atoms_str is not
present; update the method to first check whether self._default_df is not None
and atoms_str exists in self._default_df.index (or catch KeyError) and if
missing return None (or raise a clear ValueError with context), otherwise
proceed to select rows matching the stored default Name; ensure no unguarded
.loc access remains so the method never throws an uncaught KeyError.


def find_default(
self, element: Union[set[str], list[str], str]
) -> pandas.DataFrame:
"""
Find the potentials

Args:
element (set, str): element or set of elements for which you want the possible LAMMPS potentials
path (bool): choose whether to return the full path to the potential or just the potential name

Returns:
list: of possible potentials for the element or the combination of elements

"""
if isinstance(element, list):
element = set(element)
elif isinstance(element, str):
element = {element}
elif not isinstance(element, set):
raise TypeError("Only, str, list and set supported!")
element_lst = list(element)
if self._default_df is not None:
merged_lst = list(set(self._selected_atoms + element_lst))
atoms_str = "_".join(sorted(merged_lst))
return self._default_df[
(self._default_df["Name"] == self._default_df.loc[atoms_str].values[0])
]
return None
Comment on lines +194 to +221
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add error handling for missing default potential.

Line 219 has the same issue as the default() method - accessing self._default_df.loc[atoms_str] without checking if the key exists in the index.

Apply this diff:

     def find_default(
         self, element: Union[set[str], list[str], str]
     ) -> pandas.DataFrame:
         """
         Find the potentials
 
         Args:
             element (set, str): element or set of elements for which you want the possible LAMMPS potentials
             path (bool): choose whether to return the full path to the potential or just the potential name
 
         Returns:
             list: of possible potentials for the element or the combination of elements
 
         """
         if isinstance(element, list):
             element = set(element)
         elif isinstance(element, str):
             element = {element}
         elif not isinstance(element, set):
             raise TypeError("Only, str, list and set supported!")
         element_lst = list(element)
         if self._default_df is not None:
             merged_lst = list(set(self._selected_atoms + element_lst))
             atoms_str = "_".join(sorted(merged_lst))
+            if atoms_str not in self._default_df.index:
+                return None
             return self._default_df[
                 (self._default_df["Name"] == self._default_df.loc[atoms_str].values[0])
             ]
         return None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def find_default(
self, element: Union[set[str], list[str], str]
) -> pandas.DataFrame:
"""
Find the potentials
Args:
element (set, str): element or set of elements for which you want the possible LAMMPS potentials
path (bool): choose whether to return the full path to the potential or just the potential name
Returns:
list: of possible potentials for the element or the combination of elements
"""
if isinstance(element, list):
element = set(element)
elif isinstance(element, str):
element = {element}
elif not isinstance(element, set):
raise TypeError("Only, str, list and set supported!")
element_lst = list(element)
if self._default_df is not None:
merged_lst = list(set(self._selected_atoms + element_lst))
atoms_str = "_".join(sorted(merged_lst))
return self._default_df[
(self._default_df["Name"] == self._default_df.loc[atoms_str].values[0])
]
return None
def find_default(
self, element: Union[set[str], list[str], str]
) -> pandas.DataFrame:
"""
Find the potentials
Args:
element (set, str): element or set of elements for which you want the possible LAMMPS potentials
path (bool): choose whether to return the full path to the potential or just the potential name
Returns:
list: of possible potentials for the element or the combination of elements
"""
if isinstance(element, list):
element = set(element)
elif isinstance(element, str):
element = {element}
elif not isinstance(element, set):
raise TypeError("Only, str, list and set supported!")
element_lst = list(element)
if self._default_df is not None:
merged_lst = list(set(self._selected_atoms + element_lst))
atoms_str = "_".join(sorted(merged_lst))
if atoms_str not in self._default_df.index:
return None
return self._default_df[
(self._default_df["Name"] == self._default_df.loc[atoms_str].values[0])
]
return None
🧰 Tools
🪛 Ruff (0.14.3)

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

(TRY003)

🤖 Prompt for AI Agents
In pyiron_lammps/potential.py around lines 194 to 221, the code accesses
self._default_df.loc[atoms_str] without verifying that atoms_str is a valid
index key; update the logic to check whether atoms_str exists in
self._default_df.index (or wrap the access in a try/except KeyError) before
attempting to read .loc; if the key is missing, return None (or raise a clear
KeyError with a descriptive message) instead of letting an unhandled exception
occur, ensuring behavior matches the existing default() method.


def __getitem__(self, item):
potential_df = self.find(element=item)
selected_atoms = self._selected_atoms + [item]
return LammpsPotentialFile(
potential_df=potential_df,
default_df=self._default_df,
selected_atoms=selected_atoms,
resource_path=self._resource_path,
)


class PotentialAvailable:
def __init__(self, list_of_potentials):
self._list_of_potentials = {
"pot_" + v.replace("-", "_").replace(".", "_"): v
for v in list_of_potentials
}

def __getattr__(self, name):
if name in self._list_of_potentials:
return self._list_of_potentials[name]
else:
raise AttributeError

def __dir__(self):
return list(self._list_of_potentials.keys())

def __repr__(self):
return str(dir(self))


def find_potential_file_base(path, resource_path_lst, rel_path):
if path is not None:
for resource_path in resource_path_lst:
path_direct = os.path.join(resource_path, path)
path_indirect = os.path.join(resource_path, rel_path, path)
if os.path.exists(path_direct):
return path_direct
elif os.path.exists(path_indirect):
return path_indirect
raise ValueError(
"Either the filename or the functional has to be defined.",
path,
resource_path_lst,
)


def view_potentials(structure: Atoms, resource_path: str) -> pandas.DataFrame:
"""
List all interatomic potentials for the given atomistic structure including all potential parameters.

To quickly get only the names of the potentials you can use `list_potentials()` instead.

Args:
structure (Atoms): The structure for which to get potentials.
resource_path (str): Path to the "lammps/potentials_lammps.csv" file

Returns:
pandas.Dataframe: Dataframe including all potential parameters.
"""
list_of_elements = set(structure.get_chemical_symbols())
return LammpsPotentialFile(resource_path=resource_path).find(list_of_elements)


def convert_path_to_abs_posix(path: str) -> str:
"""
Convert path to an absolute POSIX path

Args:
path (str): input path.

Returns:
str: absolute path in POSIX format
"""
return (
Path(path.strip())
.expanduser()
.resolve()
.absolute()
.as_posix()
.replace("\\", "/")
)


def update_potential_paths(
df_pot: pandas.DataFrame, resource_path: str
) -> pandas.DataFrame:
config_lst = []
for row in df_pot.itertuples():
potential_file_lst = row.Filename
potential_file_path_lst = [
os.path.join(resource_path, f) for f in potential_file_lst
]
potential_dict = {os.path.basename(f): f for f in potential_file_path_lst}
potential_commands = []
for line in row.Config:
line = line.replace("\n", "")
for key, value in potential_dict.items():
line = line.replace(key, value)
potential_commands.append(line)
config_lst.append(potential_commands)
df_pot["Config"] = config_lst
return df_pot


def get_resource_path_from_conda(
env_variables: tuple[str] = ("CONDA_PREFIX", "CONDA_DIR"),
) -> str:
env = os.environ
for conda_var in env_variables:
if conda_var in env:
resource_path = os.path.join(env[conda_var], "share", "iprpy")
if os.path.exists(resource_path):
return resource_path
raise ValueError("No resource_path found" + potential_installation)


def get_potential_dataframe(structure: Atoms, resource_path=None):
if resource_path is None:
resource_path = get_resource_path_from_conda()
return update_potential_paths(
df_pot=view_potentials(structure=structure, resource_path=resource_path),
resource_path=resource_path,
)


def get_potential_by_name(potential_name: str, resource_path=None):
if resource_path is None:
resource_path = get_resource_path_from_conda()
df = LammpsPotentialFile(resource_path=resource_path).list()
return update_potential_paths(
df_pot=df[df.Name == potential_name], resource_path=resource_path
).iloc[0]
Comment on lines +349 to +355
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add validation before accessing filtered result.

Line 354 filters by potential_name and immediately accesses .iloc[0] without checking if any results were found. If no matching potential exists, this will raise an IndexError instead of a meaningful error message.

Apply this diff:

 def get_potential_by_name(potential_name: str, resource_path=None):
     if resource_path is None:
         resource_path = get_resource_path_from_conda()
     df = LammpsPotentialFile(resource_path=resource_path).list()
-    return update_potential_paths(
-        df_pot=df[df.Name == potential_name], resource_path=resource_path
-    ).iloc[0]
+    filtered_df = df[df.Name == potential_name]
+    if len(filtered_df) == 0:
+        raise ValueError(f"Potential '{potential_name}' not found in database.")
+    return update_potential_paths(
+        df_pot=filtered_df, resource_path=resource_path
+    ).iloc[0]
🤖 Prompt for AI Agents
pyiron_lammps/potential.py around lines 349 to 355: the function filters df by
potential_name and directly returns .iloc[0], which will raise IndexError if no
match is found; modify the function to check if the filtered dataframe is empty
before accessing iloc[0], and if empty raise a clear ValueError (or custom
exception) stating the requested potential_name was not found and include either
a short list of available potential names or the resource_path used for lookup
so callers can diagnose the issue; if not empty, proceed to return the first row
as before.



def validate_potential_dataframe(
potential_dataframe: pandas.DataFrame,
) -> pandas.DataFrame:
if isinstance(potential_dataframe, pandas.Series):
return potential_dataframe
elif isinstance(potential_dataframe, pandas.DataFrame):
if len(potential_dataframe) == 1:
return potential_dataframe.iloc[0]
elif len(potential_dataframe) == 0:
raise ValueError(
"The potential_dataframe is an empty pandas.DataFrame:",
potential_dataframe,
)
else:
raise ValueError(
"The potential_dataframe contains more than one interatomic potential, please select one:",
potential_dataframe,
)
else:
raise TypeError(
"The potential_dataframe should be a pandas.DataFrame or pandas.Series, but instead it is of type:",
type(potential_dataframe),
)
Loading
Loading