Give option to exclude compiling internal UMFPACK in cmake

[juhanikataja]

Elmer internal UMFPACK is not needed on supercomputing platforms and may cause problems with picky compiler stacks.

There should be a cmake build variable e.g. DISABLE_INTERNAL_UMFPACK which would disable compiling UMFPACK.

Changes would probably include:

• Change #define HAVE_UMFPACK in fem/config.h.cmake to #cmakedefine @HAVE_UMFPACK@
• Modify CMakeLists.txt in few places

[mmuetzel]

I hope you don't mind me asking which issues those are.
Do these issues still occur if you build with a current version of UMFPACK from SuiteSparse and configure with -DEXTERNAL_UMFPACK=ON?

[mmuetzel]

The changes from #579 should allow building without UMFPACK.

[juhanikataja]

It's not actually clear what is causing the incompatibility. The passage about picky compilers was weird humor that has no place in github issues, sorry about that.

The reasoning about giving an option to disable internal umfpack even without external is that

• On a supercomputer, there is no point in having a non-parallel direct solver.
• The internal UMFPACK solver in the elmer source tree adds to increased compile time, complexity and a source for weird compilation failures.
• The internal UMFPACK version is old, newer versions are faster so for production runs one should anyhow have their own UMFPACK.
• The internal UMFPACK shouldn't be completely removed since it enables Elmer to be a fully fledged FE solver without much external dependenices.

Do we assume internally in some parts of elmer the existence of UMFPACK or is it just in the solvers?

Currently there is a crude workaround at 778d9a3 where this issue stemmed from:

In file included from .../elmerfem/umfpack/src/umfpack/umf_analyze.c:29:
In file included from .../elmerfem/umfpack/src/umfpack/include/umf_internal.h:88:
In file included from .../elmerfem/umfpack/src/umfpack/../amd/amd_internal.h:72:
In file included from /usr/include/stdlib.h:394:
In file included from /usr/include/sys/types.h:29:
/usr/include/bits/types.h:155:34: error: typedef redefinition with different types ('struct __fsid_t' vs 'struct __fsid_t')
typedef struct { int __val[2]; } __fsid_t;
                                 ^
/usr/include/bits/types.h:155:26: note: previous definition is here
__STD_TYPE __FSID_T_TYPE __fsid_t;      /* Type of file system IDs.  */

[juharu]

There is an unconditional dependence to umfpack within VankaCreate.F90 in "CircutPrec()" , i guess this could be relatively easily replaced by something more general... Nothing else springs to mind atm...

[mmuetzel]

Thank you for clarifying.

True. UMFPACK (even the current upstream version) relies heavily on the respective BLAS and LAPACK implementations for parallelization. It isn't multi-threaded by itself if I understand correctly.

The compilation error you are seeing is odd. It looks like it is complaining about two conflicting type definitions in the same header?? That is odd indeed.
Can you share that header file (/usr/include/bits/types.h)? Maybe some incompatible combination of preprocessor definitions are set?

[juharu]

* On a supercomputer, there is no point in having a non-parallel direct solver. This might also not always be so, even if you are running MPI parallel, each task might have something to solve partitionwise (as in "CircuitPrec()"), for example. Thinking about it, I perhaps would be reluctant to not to have an always available robust linear solver for elmer solver...

[tzwinger]

Am I mistaken to conclude that the conflicting type definition is something platform-specific and not able to be fixed in what comes to the umfpack included in Elmer? We experience the error on a HPE-Cray system with the Cray-CCE compiler set.

[mmuetzel]

I'm not sure I understand the compiler error message correctly. It looks like it is referring to a conflicting type definition. But the locations that the error message presents look like they are from the same file and even the same line in that file.
However, it presents different content of that line. I honestly don't know what that means.
Is this a compiler error? Or some issue in glibc(?) heeders (missing inclusion guards?)? Or something completely different?

It might be that the line that it presents looks different because it is pre-processed for one and the "raw" source code for the other?

Maybe for some reason that header is included twice (and the inclusion guard isn't working for some reason).
Iiuc, headers are treated slightly differently when they are included via the system include path or via a user include path. Maybe, the header somehow ends up being included via both and the compiler doesn't "realize" it is the same???
If that is the case, compiling with -std=gnu11 (or whatever flag the Cray compiler needs for C11). C11 should allow multiple typedefs if they are compatible afaict.

Does the issue go away if you add that flag to CMAKE_C_FLAGS when configuring ElmerFEM?

[juhanikataja]

The problem appears to be in the combination of specific cray programming environment modules where -fopenmp flag looks like it is injecting typedef ... __fsid_t in the global scope.

Since we don't hopefully need fsid_t, I suggest we just use the workaround 778d9a3 mentioned earlier in devel.

The following code reproduces the bug with cc -fopenmp bug.c when those modules are loaded:

typedef struct { int __val[2]; } __fsid_t;
typedef __fsid_t fsid_t;

int main(int argc, char** argv) {
  return 0;
}

[mmuetzel]

This issue seems to be caused by the fact that the build rules of UMFPACK (and AMD) are preprocessing the original sources with a different set of flags than when compiling. The situation with the Cray compiler might not be the only affected build toolchain. (Theoretically, doing that could result in a "successful" compilation but undefined (or unexpected) behavior on runtime.)

The changes proposed in #587 avoid the separate preprocessing step entirely by using CMake object libraries to build parts of the sources for the "actual" libraries with different sets of preprocessor flags.

Do you still see the issues from here with the changes from PR #587?