-
Notifications
You must be signed in to change notification settings - Fork 284
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 nwchem easyblock to handle different versions of OpenMPI. #1104
Update nwchem easyblock to handle different versions of OpenMPI. #1104
Conversation
Also fix missing libs in linking when building with OpenIB as armci network.
easybuild/easyblocks/n/nwchem.py
Outdated
|
||
for var in ['USE_MPI', 'USE_MPIF', 'USE_MPIF4']: | ||
env.setvar(var, 'y') | ||
for var in ['CC', 'CXX', 'F90']: | ||
env.setvar('MPI_%s' % var, os.getenv('MPI%s' % var)) | ||
#if LooseVersion(self.version) <= LooseVersion("6.5"): |
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 remove?
easybuild/easyblocks/n/nwchem.py
Outdated
@@ -199,6 +208,9 @@ def configure_step(self): | |||
libmpi = "-lmpichf90 -lmpich -lopa -lmpl -lrt -lpthread" | |||
else: | |||
raise EasyBuildError("Don't know how to set LIBMPI for %s", mpi_family) | |||
if self.cfg['armci_network'] in ["OPENIB"]: | |||
libmpi += " -libumad -libverbs -lpthread" | |||
#if LooseVersion(self.version) <= LooseVersion("6.5"): |
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 remove?
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.
That #if Looseversion (both of them) should probably be activated but i haven't had time to verify that it works if i do...
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.
so, $LIBMPI
doesn't need to be set anymore for NWChem 6.6 and newer?
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.
Thats what their docco says, unfortunately that probably destroys the ARMCI fix, so i haven't taken the time to test that.
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.
Do you have a reference to the documentation?
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.
confirmed, no need to set these environment variables anymore with NWChem 6.6, see akesandgren#3
easybuild/easyblocks/n/nwchem.py
Outdated
else: | ||
libmpi = "-lmpi_usempi -lmpi_mpifh -lmpi" | ||
else: | ||
libmpi = "-lmpi_usempif08 -lmpi_usempi_ignore_tkr -lmpi_mpifh -lmpi" |
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.
oh boy, what a mess :)
do you have additional easyconfigs to test these changes @akesandgren?
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.
Yes i do have an easyconfig that need that fix, i.e. using a iomkl/2017.01 which uses OpenMPI/2.0.1
It dies horribly for us when using intel/2017.x so openmpi was the way to go.
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.
can you open a pull request for that?
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.
Will do...
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 accidentally tried to build my new nwchem 6.6 using iomkl/2017a (openmpi 2.0.2) with the old nwchem.py and it subsequently failed. Using this new nwchem.py worked though, so it got "properly" tested ...
For NWChem 6.6 and newer, $LIBMPI & co should no longer be set, the correct values are determined by the NWChem build procedure automatically, see http://www.nwchem-sw.org/index.php/Compiling_NWChem#MPI_variables Also fix a minor problem with libmpi, must not be "None" when it is used in setting BLASOPT.
lgtm, I'll retest with existing NWChem easyconfigs and easybuilders/easybuild-easyconfigs#4231, but should be good to go, thanks @akesandgren! |
easybuild/easyblocks/n/nwchem.py
Outdated
# set, the correct values are determined by the NWChem build | ||
# procedure automatically, see | ||
# http://www.nwchem-sw.org/index.php/Compiling_NWChem#MPI_variables | ||
if LooseVersion(self.version) <= LooseVersion("6.5"): |
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.
@akesandgren We'll need to make this "< 6.6", since otherwise the build of NWChem version 6.5.revision26243 is broken:
>>> from distutils.version import LooseVersion
>>> LooseVersion('6.5.revision26243') <= LooseVersion('6.5')
False
>>> LooseVersion('6.5.revision26243') < LooseVersion('6.6')
True
>>> from distutils.version import LooseVersion >>> LooseVersion('6.5.revision26243') <= LooseVersion('6.5') False >>> LooseVersion('6.5.revision26243') < LooseVersion('6.6') True
test changed to < 6.6 |
verified with existing NWChem easyconfigs and easybuilders/easybuild-easyconfigs#4231, good to go Thanks @akesandgren! |
Also fix missing libs in linking when building with OpenIB as armci
network.
And turn on USE_NOIO by default.