MPI.h imports system mpi.h with no check against linked MPI, HAS_MPI must be set manually.

[Andrey Shmakov]

ISSUE: importing dolfin/common/MPI.h (imported automatically by dolfin.h) has preprocessor directive which uses definition that is only set at Dolfin Compile time. This breaks programs linking dolfin that chose to use other MPI libraries internally or no MPI.

EXAMPLE: attached file

#include <dolfin.h>
#include <stdio.h>

int main(){
  if(dolfin::has_mpi()){ //Will be true if Dolfin was linked against MPI
    printf ("HAS_MPI_DEFINED");
    printf ("%u", MPI_COMM_WORLD); //Will report integer unless HAS_MPI is defined
    printf ("%u",dolfin::MPI::rank(MPI_COMM_WORLD)); //Will fail unless this file is linked -lmpi -lmpi_cxx and HAS_MPI is defined
    // Additionally, -lmpi and -lmpi_cxx MUST point to the same MPI implementation linked by dolfin
  }
  else
    printf ("NO MPI DEFINED");

  return 0;
}

Code Snippet that will fail (with no explicit reference to MPI_COMM_WORLD or mpi at all) in external application linking dolfin.

dolfin::File test_file("test");
test_file << *some_dolfin_object

EXPLANATION: This will fail to link unless application is explicitly linked against libmpi and libmpi_cxx, dolfin::has_mpi() is correctly linked against MPI library used by dolfin at build time (and will return true). However MPI_COMM_WORLD is set in dolfin/common/MPI.h based on preprocessor definition HAS_MPI. If no MPI library is used by application linking DOLFIN, MPI.h sets MPI_COMM_WORLD to an integer. If Dolfin is linked against OpenMPI or mpich2, type MPI_Comm is not an integer, and basic functions (like File creation in dolfin/io/File.h) will fail to link with error " undefined reference to `dolfin::MPI::rank(int)". Additionally if HAS_MPI is set, dolfin/common/MPI.h includes <mpi.h> based on whatever is in the include path of the external application being built, and is not guaranteed to be the same MPI implementation linked by dolfin when built which will break MPI as well.

Related Source:

dolfin/common/MPI.h

#ifdef HAS_MPI
#define MPICH_IGNORE_CXX_SEEK 1
#include <mpi.h>
#endif

#include <dolfin/log/dolfin_log.h>

#ifndef HAS_MPI
typedef int MPI_Comm;
#define MPI_COMM_WORLD 2
#define MPI_COMM_SELF 1
#define MPI_COMM_NULL 0
#endif

[Prof Garth Wells]

The necessary compiler definitions for building DOLFIN are in DOLFINConfig.cmake.

[Andrey Shmakov]

[deleted]

[Andrey Shmakov]

This is true, but it will break applications that do not use MPI, libdolfin.so can be calling the libmpi it wants for all I care, file output, which is a linked libdolfin function, should not be relying on preprocessor definitions set by my external application in dolfin headers. Additionally if you are requiring third parties use cmake this should be explicitly stated, meaning that is very unclear that these preprocessor defines are make-or-break for linking against dolfin.

I am linking libdolfin, not libmpi, and libdolfin.so should be loading the libraries it needs for basic functions/type definitions without requiring me to share preprocessor directives and explicitly link against other libraries that I am not using.

[Andrey Shmakov]

My point is that internal Dolfin MPI handling should not rely on header imports and preprocessor directives, but be loaded explictly by libdolfin.so when the application loads libdolfin, which is not the case right now.

[Andrey Shmakov]

My misunderstanding of the Dolfin build decision, too big of a change to request as a bug/issue to make Dolfin standalone.

[Prof Garth Wells]

The reason for the typedefs is to allow users to write code that works with or without MPI. We decided to not wrap MPI communicators (see https://www.mail-archive.com/fenics%40fenicsproject.org/msg00715.html).

Maybe it is better to introduce DOLFIN_COMM_SELF and DOLFIN_COMM_WORLD, etc.

To dispense with HAS_MPI, we could need to hardwire the typedefs at configure time.