Make Intel compiler + IntelMPI use mpiifort etc.#2005

akesandgren · 2016-11-18T08:24:58Z

The combination of Intel compilers and IntelMPI should always use
mpiifort, mpiicc, mpiicpc and not mpif90, mpicc, mpicxx.

The main reason is that for Fortran90 code that uses
USE mpi
mpif90 finds the gfortran mpi.mod and not the ifort one, this regardless
of having I_MPI_F77/F90/FC set to ifort or not.

Without this patch building of any fortran mpi code that uses mpi.mod or
any of the other Fortran modules fails.

It is also a recommendation by Intel to use mpiicc, mpiicpc, and
mpiifort for this combination and not to use mpif90 with I_MPI_F90 or
similar.

Longer discussion about this in issue #2000

The combination of Intel compilers and IntelMPI should always use mpiifort, mpiicc, mpiicpc and not mpif90, mpicc, mpicxx. The main reason is that for Fortran90 code that uses USE mpi mpif90 finds the gfortran mpi.mod and not the ifort one, this regardless of having I_MPI_F77/F90/FC set to ifort or not. Without this patch building of any fortran mpi code that uses mpi.mod or any of the other Fortran modules fails. It is also a recommendation by Intel to use mpiicc, mpiicpc, and mpiifort for this combination and not to use mpif90 with I_MPI_F90 or similar.

fgeorgatos · 2016-11-23T09:26:28Z

tagging @besserox for further comment, in case there is one.

boegel · 2016-11-23T14:32:57Z

Although I can't come up with a reason not to make this change, this is a backward-incompatible change...
Saying that this shouldn't have any negative side-effects (i.e. builds breaking that work fine with mpif90 + $I_MPI_F90) would be optimistic, since the exact reason we're changing this is because what we have now doesn't work (so switching has a clear consequence).

However, I see this as a bug fix, so making a backward-incompatible change like this may be acceptable.

Thoughts on this change @ocaisa, @damianam, @geimer, @besserox (taking into account the discussion in #2000)?

@akesandgren Can you look into the failing test that Travis is reporting? There's only one, which has hardcoded checks on the MPI compiler command being used, so fixing should be trivial:

FAIL: Test for ictce toolchain.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/hpcugent/easybuild-framework/test/framework/toolchain.py", line 537, in test_ictce_toolchain
    self.assertEqual(tc.get_variable('CC'), 'mpicc')
  File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/vsc_install-0.10.19-py2.6.egg/vsc/install/testing.py", line 79, in assertEqual
    raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF])))
AssertionError: 'mpiicc' != 'mpicc':
DIFF:
- mpiicc?    -

You can run this test in isolation in your working copy using python test/framework/toolchain.py test_ictce_toolchain.

ocaisa · 2016-11-23T15:42:18Z

easybuild/toolchains/mpi/mpich2.py

-        self.MPI_COMPILER_MPIF90 = 'mpif90'
-        self.MPI_COMPILER_MPIFC = 'mpif90'
+        if self.MPI_COMPILER_MPIF77 is None and self.MPI_COMPILER_MPIF90 is None and self.MPI_COMPILER_MPIFC is None:
+            # hardwire MPI wrapper commands (otherwise Mpich parent class sets them based on MPICH version)


Shouldn't we add MPI{CC, CXX} to be consistent...or explain in the comment why you don't have to

@ocaisa only MPI{F77,F90,FC} are being touched here, but I guess it doesn't hurt to clarify

akesandgren · 2016-11-24T11:32:21Z

Future proofing the tests should probably modify all checks of mpicc, et.al., to have a intelcompiler+intelmpi combo switch, but i'll test only in ictce to start with.

boegel · 2016-11-24T11:53:56Z

@akesandgren seeing the tests fail when a backwards-incompatible change is being made is actually a good thing :)

akesandgren · 2016-11-24T12:27:26Z

Added changes to the test_ictce_toolchain to match the rest of the changes.

boegel · 2016-11-24T13:46:48Z

lgtm, but I'd like to get feedback on this from @geimer and @wpoely86

geimer · 2016-11-24T15:54:08Z

@boegel Going with mpiicc, mpiicpc, and mpiifort for Intel MPI with Intel compilers makes sense to me. That's also what I would choose as a user.

I haven't had the chance to test the code in this PR yet, though the changes look reasonable.

akesandgren · 2016-11-24T16:03:45Z

For reference, I'm using it it production and so far all good :-)

boegel · 2016-11-24T16:07:40Z

@akesandgren which toolchains have you tested this with, which software? Any chance for a list of installed modules (just (filtered) output from find . -type f is fine)

akesandgren · 2016-11-24T16:14:50Z

I'm only using intel/2017.01 at the moment (well a bit of foss/2016b too but that's irrelevant for this thread).
Built top level SW is:
OpenFOAM/2.4.0 QuantumESPRESSO/5.4.0-hybrid QuantumESPRESSO/6.0-hybrid VASP/5.4.1-05Feb16-p02-hpc2n DALTON/2016.2-hybrid HDF5/1.8.17

Haven't had time to build more (yet) after the rebuild-everything-from-scratch a few days ago.

wpoely86 · 2016-11-25T08:38:19Z

easybuild/toolchains/mpi/mpich2.py

-        self.MPI_COMPILER_MPIF90 = 'mpif90'
-        self.MPI_COMPILER_MPIFC = 'mpif90'
+        if self.MPI_COMPILER_MPIF77 is None and self.MPI_COMPILER_MPIF90 is None and self.MPI_COMPILER_MPIFC is None:
+            # hardwire MPI wrapper commands (otherwise Mpich parent class sets them based on MPICH version)


definitely clarify. The docstring says: Set the MPICH_{CC, CXX, F77, F90, FC} variables

OK, I guess clarifying wouldn't hurt.

@akesandgren can you include a comment that only the Fortran compiler commands need to be overridden, i.e. that the C/C++ compilers are correctly set via the parent class?

boegel · 2016-11-25T10:27:15Z

@akesandgren I'm testing this with all imkl easyconfigs that are installed with iimpi + all easyconfigs using an intel toolchain that have usempi enabled (i.e. which explicitly use the MPI compiler wrappers).

So far, so good, but I'll ride it out until those tests are completed. I don't expect any problems, but it doesn't hurt to make sure.

The plan is to include this in EasyBuild v3.0.1 bug fix release, which will probably go out mid next week.

boegel · 2016-11-26T11:35:39Z

My testing suggests that old versions of ParaView (4.1.0 w/ intel/2014b and 4.3.1 w/ intel/2015a) + PETSc (3.5.1 w/ intel/2014b and 3.5.3 w/ intel/2015a) are broken by this change.

I'll look into the details, and see how we can mitigate this in the relevant easyblocks/easyconfigs/patches...

boegel · 2016-11-26T14:49:39Z

OK, both the problems with old PETSc and ParaView versions/toolchains are red herrings. The builds failed in my tests, but not because of the switch the mpiifort & co in this PR.

PETSc failed because a very old zlib installation that was lacking version info was being picked up from somewhere else (a matching zlib module from another location listed in $MODULEPATH).

ParaView failed with error like /usr/include/GL/glxext.h(481): error: identifier "GLintptr" is undefined because due to an incompatible Mesa being picked up from the OS (the old ParaView easyconfigs don't include Mesa as a proper dep).

So, as far as I can tell, this is good to go.

Thanks for reporting this and working out a fix @akesandgren!

We'll need to mention this clearly in the release notes for EasyBuild v3.0.1 (cc @migueldiascosta).

boegel · 2016-11-29T10:06:47Z

I did find one victim of this change in further testing, but a very minor one (source is not freely available), see fix in ALADIN easyblock at easybuilders/easybuild-easyblocks#1050

boegel added this to the 3.0.1 milestone Nov 23, 2016

ocaisa reviewed Nov 23, 2016

View reviewed changes

akesandgren added 2 commits November 24, 2016 13:25

Adjust test_ictce_toolchain to use mpiicc, mpiicpc, mpiifort.

b8d62ad

Merge branch 'develop' into make-intel+impi-toolchain-use-mpiifort-etc

74251d5

wpoely86 approved these changes Nov 25, 2016

View reviewed changes

Add comment about C/CXX wrappers in mpich2

5a1550d

boegel merged commit 5fde304 into easybuilders:develop Nov 26, 2016

boegel mentioned this pull request Nov 26, 2016

Intel MPI with Intel Fortran compiler must use mpiifort to be fully usable #2000

Closed

akesandgren deleted the make-intel+impi-toolchain-use-mpiifort-etc branch November 29, 2016 07:29

boegel mentioned this pull request Nov 29, 2016

always specify name of serial Fortran compiler to ALADIN, it already knows to use MPI wrapper commands easybuilders/easybuild-easyblocks#1050

Merged

boegel mentioned this pull request Dec 1, 2016

ncview Intel compiler issue EB 2.X/3.X easybuilders/easybuild-easyconfigs#3880

Closed

Conversation

akesandgren commented Nov 18, 2016

Uh oh!

fgeorgatos commented Nov 23, 2016

Uh oh!

boegel commented Nov 23, 2016

Uh oh!

ocaisa Nov 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel Nov 23, 2016

Choose a reason for hiding this comment

Uh oh!

akesandgren commented Nov 24, 2016

Uh oh!

boegel commented Nov 24, 2016

Uh oh!

akesandgren commented Nov 24, 2016

Uh oh!

boegel commented Nov 24, 2016

Uh oh!

geimer commented Nov 24, 2016

Uh oh!

akesandgren commented Nov 24, 2016

Uh oh!

boegel commented Nov 24, 2016

Uh oh!

akesandgren commented Nov 24, 2016

Uh oh!

wpoely86 Nov 25, 2016

Choose a reason for hiding this comment

Uh oh!

boegel Nov 25, 2016

Choose a reason for hiding this comment

Uh oh!

boegel commented Nov 25, 2016

Uh oh!

boegel commented Nov 26, 2016

Uh oh!

boegel commented Nov 26, 2016

Uh oh!

boegel commented Nov 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ocaisa Nov 23, 2016 •

edited

Loading