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

enhance Amber easyblock for AmberTools #2618

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
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
18 changes: 13 additions & 5 deletions easybuild/easyblocks/a/amber.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,21 @@ def configure_step(self):
external_libs_list.append('fftw')
if get_software_root('netCDF'):
external_libs_list.append('netcdf')
if get_software_root('netCDF-Fortran'):
netcdf_fortran_root = get_software_root('netCDF-Fortran')
if netcdf_fortran_root:
external_libs_list.append('netcdf-fortran')
self.cfg.update('configopts', '-DNetCDF_INCLUDES=%s/include' % netcdf_fortran_root)
if get_software_root('zlib'):
external_libs_list.append('zlib')
if get_software_root('Boost'):
external_libs_list.append('boost')
if get_software_root('PnetCDF'):
external_libs_list.append('pnetcdf')

# Force to use BLAS and LAPACK from environment variables
self.cfg.update('configopts', '-DBLAS_LIBRARIES="%s"' % os.getenv('LIBBLAS'))
self.cfg.update('configopts', '-DLAPACK_LIBRARIES="%s"' % os.getenv('LIBLAPACK'))

# Force libs for available deps (see cmake/3rdPartyTools.cmake in Amber source for list of 3rd party libs)
# This provides an extra layer of checking but should already be handled by TRUST_SYSTEM_LIBS=TRUE
external_libs = ";".join(external_libs_list)
Expand All @@ -187,9 +193,11 @@ def configure_step(self):

# Amber recommend running the tests from the sources, rather than putting in installation dir
# due to size. We handle tests under the install step
self.cfg.update('configopts', '-DINSTALL_TESTS=FALSE')
if "-DINSTALL_TESTS=" not in self.cfg['configopts']:
self.cfg.update('configopts', '-DINSTALL_TESTS=FALSE')

self.cfg.update('configopts', '-DCOMPILER=AUTO')
if "-DCOMPILER=" not in self.cfg['configopts']:
self.cfg.update('configopts', '-DCOMPILER=AUTO')

# configure using cmake
super(EB_Amber, self).configure_step()
Expand Down Expand Up @@ -315,8 +323,8 @@ def install_step(self):

super(EB_Amber, self).install_step()

# Run the tests located in the build directory
if self.cfg['runtest']:
# Run the tests located in the build directory, only for Amber. Tests fail when building only AmberTools
if self.cfg['runtest'] and self.name == 'Amber':
Copy link
Contributor

Choose a reason for hiding this comment

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

do all tests fail with AmberTools, or only some?
if not all tests fail, I think it's still useful to run them in combination with EB option --ignore-test-failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not try to untangle the tests, but not all executables from Amber get created, and the tests assume that they are there.

Copy link
Member

Choose a reason for hiding this comment

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

See #2604 (and easybuilders/easybuild-easyconfigs#14028) - it is trying to run the Amber test suite and not the AmberTools test suite. There were still some failures with the AmberTool test suite and I've not had the chance to investigate further.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we're doing the runtest part wrong. I think I have the correct stuff in my test-.eb but I got sidetracked and haven't had time to go back and revisit.
It should not matter if we build Amber or AmberTools the test part (CMake and old style) is written so that it only tests what is actually built, if we call it correctly. I checked with the devs on that.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akesandgren did you hear back from the Amber devs yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

They basically said what I said above, The CMake part should do the right thing when asked to run make test.
But I haven't had time to revisit this on my side...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This still fails as of today. Can't build AmberTools without disabling tests.

pretestcommands = 'source %s/amber.sh && cd %s/test' % (self.installdir, self.builddir)

# serial tests
Expand Down