-
Notifications
You must be signed in to change notification settings - Fork 285
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
update GAMESS-US easyblock to directly write install.info #2791
update GAMESS-US easyblock to directly write install.info #2791
Conversation
This is a working EasyBlock for recent releases of
|
line = re.sub(r"(null\) set VERNO)=.*", r"\1=%s" % self.version, line) | ||
line = re.sub(r"^(\s*set DDI_MPI_CHOICE)=.*", r"\1=%s" % mpilib, line) | ||
line = re.sub(r"^(\s*set DDI_MPI_ROOT)=.*%s.*" % mpilib.lower(), r"\1=%s" % mpilib_path, line) | ||
line = re.sub(r"^(\s*set GA_MPI_ROOT)=.*%s.*" % mpilib.lower(), r"\1=%s" % mpilib_path, line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined name 'mpilib'
undefined name 'mpilib_path'
line = re.sub(r"^(\s*set\s*GMS_OPENMP)=.*", r"\1=%s" % "true", line) # changed in gamess-20190930-R2 for OpenBLAS support | ||
line = re.sub(r"(null\) set VERNO)=.*", r"\1=%s" % self.version, line) | ||
line = re.sub(r"^(\s*set DDI_MPI_CHOICE)=.*", r"\1=%s" % mpilib, line) | ||
line = re.sub(r"^(\s*set DDI_MPI_ROOT)=.*%s.*" % mpilib.lower(), r"\1=%s" % mpilib_path, line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined name 'mpilib'
undefined name 'mpilib_path'
line = re.sub(r"^(\s*set\s*GMSPATH)=.*", r"\1=%s\n%s" % (self.installdir, extra_gmspath_lines), line) | ||
line = re.sub(r"^(\s*set\s*GMS_OPENMP)=.*", r"\1=%s" % "true", line) # changed in gamess-20190930-R2 for OpenBLAS support | ||
line = re.sub(r"(null\) set VERNO)=.*", r"\1=%s" % self.version, line) | ||
line = re.sub(r"^(\s*set DDI_MPI_CHOICE)=.*", r"\1=%s" % mpilib, line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined name 'mpilib'
for line in fileinput.input(rungms, inplace=1, backup='.orig'): | ||
line = re.sub(r"^(\s*set\s*TARGET)=.*", r"\1=%s" % self.cfg['ddi_comm'], line) | ||
line = re.sub(r"^(\s*set\s*GMSPATH)=.*", r"\1=%s\n%s" % (self.installdir, extra_gmspath_lines), line) | ||
line = re.sub(r"^(\s*set\s*GMS_OPENMP)=.*", r"\1=%s" % "true", line) # changed in gamess-20190930-R2 for OpenBLAS support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least two spaces before inline comment
line too long (137 > 120 characters)
raise EasyBuildError("Failed to patch actvte.code", actvte, err) | ||
# compiling | ||
run_cmd("mv %s/tools/actvte.code" % self.builddir + " %s/tools/actvte.f" % self.builddir) | ||
run_cmd("%s -o " % fortran_comp + " %s/tools/actvte.x" % self.builddir + " %s/tools/actvte.f" % self.builddir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (122 > 120 characters)
"Enter your choice of 'mkl' or .* 'none': ": mathlib, | ||
} | ||
run_cmd_qa(cmd, qa=qa, std_qa=stdqa, log_all=True, simple=True) | ||
f.writelines(["setenv GMS_MPI_LIB " + mpilib + "\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trailing whitespace
# verify selected DDI communication layer | ||
known_ddi_comms = ['mpi', 'mixed', 'shmem', 'sockets'] | ||
if not self.cfg['ddi_comm'] in known_ddi_comms: | ||
raise EasyBuildError("Unsupported DDI communication layer specified (known: %s): %s", | ||
known_ddi_comms, self.cfg['ddi_comm']) | ||
|
||
f.writelines(["setenv GMS_DDI_COMM " + self.cfg['ddi_comm'] + "\n" ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace before ']'
# verify selected DDI communication layer | ||
known_ddi_comms = ['mpi', 'mixed', 'shmem', 'sockets'] | ||
if not self.cfg['ddi_comm'] in known_ddi_comms: | ||
raise EasyBuildError("Unsupported DDI communication layer specified (known: %s): %s", | ||
known_ddi_comms, self.cfg['ddi_comm']) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line contains whitespace
f.writelines(["setenv GMS_MATHLIB " + mathlib + "\n" + | ||
"setenv GMS_MATHLIB_PATH " + mathlib_root + "\n" + | ||
"setenv GMS_MKL_VERNO " + mathlib_vers + "\n" + | ||
"setenv GMS_LAPACK_LINK_LINE" + "" + "\n"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation line under-indented for visual indent
else: | ||
mathlib = mathlib.lower() | ||
|
||
f.writelines(["setenv GMS_MATHLIB " + mathlib + "\n" + | ||
"setenv GMS_MATHLIB_PATH " + mathlib_root + "\n" + | ||
"setenv GMS_MKL_VERNO " + mathlib_vers + "\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation line under-indented for visual indent
Have a look at |
@saromleang Thanks for pointing that out to us! That looks promising. Just to confirm: any changes to the ''install.info'' script will be taken care of by running that command.So if a new field is added, the script will be automatically updated. Is that correct? |
Yes, we have a document template for install.info that is leverage by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do something along the lines of
buildopts = dict()
...
buildopts['VB2000'] = self.cfg['use_bundled_vb2000']
...
for key, val in buildopts.items():
f.write('setenv', key, val, '\n')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this approach of directly writing install.info
instead going through the question/answer game. However there are several parts of the easyblock that need improvement.
I've been carefully reviewing the code of this easyblock and I made extensive changes to it that address the main problems outlined below. Please check my changes in sassy-crick#1 and if you agree with them merge it to this PR.
else: | ||
mathlib = mathlib.lower() | ||
|
||
f.writelines(["setenv GMS_MATHLIB " + mathlib + "\n" + | ||
"setenv GMS_MATHLIB_PATH " + mathlib_root + "\n" + | ||
"setenv GMS_MKL_VERNO " + mathlib_vers + "\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mathlib_vers
is not necessarily assigned here, you only defined it for imkl
run_cmd("mv %s/tools/actvte.code" % self.builddir + " %s/tools/actvte.f" % self.builddir) | ||
run_cmd("%s -o " % fortran_comp + " %s/tools/actvte.x" % self.builddir + " %s/tools/actvte.f" % self.builddir) | ||
|
||
def build_step(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are adding a second build_step
here, I guess it's leftover code.
@@ -64,6 +67,7 @@ def extra_options(): | |||
'maxcpus': [None, "Maximum number of cores per node", MANDATORY], | |||
'maxnodes': [None, "Maximum number of nodes", MANDATORY], | |||
'runtest': [True, "Run GAMESS-US tests", CUSTOM], | |||
'omp': [False, "Enabling OpenMP", CUSTOM], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be replaced with toolchain.options.openmp
from easybuild.tools.modules import get_software_root, get_software_version | ||
from easybuild.tools.py2vs3 import ascii_letters | ||
from easybuild.tools.run import run_cmd, run_cmd_qa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run_cmd_qa
is no longer needed
@@ -88,12 +94,17 @@ def extract_step(self): | |||
|
|||
def configure_step(self): | |||
"""Configure GAMESS-US build via provided interactive 'config' script.""" | |||
|
|||
"""Opening install.info file as append""" | |||
f = open(self.builddir + "/install.info", "a") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can leverage write_file
from easybuild instead of manually opening and closing the file handler
@sassy-crick ping on this one... |
@sassy-crick your commits are now part of #3047 where we are continuing this PR. Closing as superseded by #3047 |
(created using
eb --new-pr
)