Skip to content
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

Add JDFTx jobs.py to run JDFTx using atomate2/jobflow #349

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions src/custodian/jdftx/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"""
This package implements various JDFTx Jobs and Error Handlers.
Used Cp2kJob developed by Nick Winner as a template.
"""
86 changes: 86 additions & 0 deletions src/custodian/jdftx/jobs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
"""This module implements basic kinds of jobs for JDFTx runs."""

import logging
import os
import subprocess

from custodian.custodian import Job

logger = logging.getLogger(__name__)


class JDFTxJob(Job):
"""A basic JDFTx job. Runs whatever is in the working directory."""

# If testing, use something like:
# job = JDFTxJob()
# job.run() # assumes input files already written to directory

def __init__(
self,
jdftx_cmd,
input_file="jdftx.in",
output_file="jdftx.out",
stderr_file="std_err.txt",
) -> None:
"""
This constructor is necessarily complex due to the need for
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove the note about the constructor being "complex", it doesn't seem to be complex. I believe this note is copy-pasted from another constructor that is complex

flexibility. For standard kinds of runs, it's often better to use one
of the static constructors. The defaults are usually fine too.

Args:
jdftx_cmd (str): Command to run JDFTx as a string.
input_file (str): Name of the file to use as input to JDFTx
executable. Defaults to "input.in"
output_file (str): Name of file to direct standard out to.
Defaults to "jdftx.out".
stderr_file (str): Name of file to direct standard error to.
Defaults to "std_err.txt".
"""
self.jdftx_cmd = jdftx_cmd
self.input_file = input_file
self.output_file = output_file
self.stderr_file = stderr_file

def setup(self, directory="./") -> None:
"""No setup required."""

def run(self, directory="./"):
"""
Perform the actual JDFTx run.

Returns:
-------
(subprocess.Popen) Used for monitoring.
"""
cmd = self.jdftx_cmd + " -i " + self.input_file + " -o " + self.output_file
logger.info(f"Running {cmd}")
with (
open(os.path.join(directory, self.output_file), "w") as f_std,
open(os.path.join(directory, self.stderr_file), "w", buffering=1) as f_err,
):
# use line buffering for stderr
return subprocess.run(
cmd.split(),
Copy link
Member

Choose a reason for hiding this comment

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

should prefer shlex.split()

cwd=directory,
stdout=f_std,
stderr=f_err,
shell=False,
check=False,
)

def postprocess(self, directory="./") -> None:
"""No post-processing required."""

def terminate(self, directory="./") -> None:
"""Terminate JDFTx."""
# This will kill any running process with "jdftx" in the name,
# this might have unintended consequences if running multiple jdftx processes
# on the same node.
for cmd in self.jdftx_cmd:
if "jdftx" in cmd:
try:
os.system(f"killall {cmd}")
Copy link
Member

Choose a reason for hiding this comment

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

See implementation of terminate() in VASP job to avoid killing all JDFTx processes when possible

except Exception as e:
print(f"Unexpected error occurred: {e}")
raise
Loading