Skip to content

Use ASE_LAMMPSRUN_COMMAND as a fallback option #313

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 2 commits into
base: main
Choose a base branch
from
Open

Use ASE_LAMMPSRUN_COMMAND as a fallback option #313

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18aa5d2
Use ASE_LAMMPSRUN_COMMAND as a fallback option
jan-janssen Feb 20, 2026
0f1e6c1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 20, 2026
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
8 changes: 7 additions & 1 deletion src/lammpsparser/compatibility/file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def lammps_file_interface_function(
calc_mode: str = "static",
calc_kwargs: Optional[dict] = None,
units: str = "metal",
lmp_command: str = "mpiexec -n 1 --oversubscribe lmp_mpi -in lmp.in",
lmp_command: Optional[str] = None,
resource_path: Optional[str] = None,
input_control_file: Optional[dict] = None,
write_restart_file: bool = False,
Expand Down Expand Up @@ -93,6 +93,12 @@ def lammps_file_interface_function(
else:
calc_kwargs = calc_kwargs.copy()

if lmp_command is None:
lmp_command = (
os.getenv("ASE_LAMMPSRUN_COMMAND", "mpiexec -n 1 --oversubscribe lmp_mpi")
+ " -in lmp.in"
)
Comment on lines +96 to +100
Copy link

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

ASE_LAMMPSRUN_COMMAND="" produces a broken command silently.

os.getenv(key, default) only returns the default when the variable is absent. If the variable is set to an empty string, lmp_command becomes " -in lmp.in", which subprocess.check_output(..., shell=True) will invoke and fail with a cryptic "command not found" error.

🛠️ Proposed fix 
-    if lmp_command is None:
-        lmp_command = (
-            os.getenv("ASE_LAMMPSRUN_COMMAND", "mpiexec -n 1 --oversubscribe lmp_mpi")
-            + " -in lmp.in"
-        )
+    if lmp_command is None:
+        lmp_command = (
+            os.getenv("ASE_LAMMPSRUN_COMMAND") or "mpiexec -n 1 --oversubscribe lmp_mpi"
+        ) + " -in lmp.in"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lammpsparser/compatibility/file.py` around lines 96 - 100, The code sets
lmp_command from os.getenv("ASE_LAMMPSRUN_COMMAND", ...) which yields the
default only if the variable is absent, so an explicitly empty
ASE_LAMMPSRUN_COMMAND yields a broken value like " -in lmp.in"; update the logic
around lmp_command (the os.getenv call and its assignment) to treat an empty or
whitespace-only environment variable as unset and fall back to the intended
default ("mpiexec -n 1 --oversubscribe lmp_mpi"), e.g. read the env with
os.environ.get or os.getenv(..., None), strip and check truthiness before
concatenating " -in lmp.in", and ensure the final lmp_command used with
subprocess.check_output is a non-empty, valid command string.


os.makedirs(working_directory, exist_ok=True)
potential_lst, potential_replace, species = _get_potential(
potential=potential, resource_path=resource_path
Expand Down
Loading