Skip to content

Fix MPI-CXX dependency of PETSc#1917

Merged
akesandgren merged 2 commits intoeasybuilders:developfrom
Flamefire:petsc_ignore_mpicxx
Mar 4, 2020
Merged

Fix MPI-CXX dependency of PETSc#1917
akesandgren merged 2 commits intoeasybuilders:developfrom
Flamefire:petsc_ignore_mpicxx

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

@Flamefire Flamefire commented Jan 14, 2020

Due to SuperLU (automatically downloaded and build with PETSc) using mpicxx
the resulting libpetsc.so will have a dependency on C++ MPI symbols causing
failures in C applications linking against it even though those symbols are never used.

This removes the include to the mpi cxx extension inside openmpis mpi.h.
Downside is that all consumers of PETSc will have this define too as
PETSc saves its used command line flags.

IMO this is OK as to my knowledge nobody uses those extensions anyway, especially in the PETSc consumer list (e.g. SLEPc)

edit (by @boegel): see also easybuilders/easybuild-easyconfigs#9610

@boegel
Copy link
Copy Markdown
Member

boegel commented Jan 14, 2020

@Flamefire Can you clarify what you mean by "PETSc consumers"?

Is that anything depending on PETSc?

If so, this change will break other stuff, only to support building PETSc on top of OpenMPI configured with --enable-mpi-cxx, which is a site customization (since the OpenMPI easyconfigs in the central repo don't use that configure option).

So, we should be more careful here and make sure we apply the necessary updates to anything that uses PETSc...

@Flamefire
Copy link
Copy Markdown
Contributor Author

Yes. PETSc has a config file with flags that is used by e.g. SLEPc to build SLEPc.

The problem is not that easy: The goal is to have a PETSc which does not depend on mpi-cxx. Building PETSc is fine without this. But (especially C) consumers fail to try-link against PETSc due to the mpi-cxx dependency which is unnecessarily pulled in by PETSc through SuperLU through OpenMPIs mpi.h

So yes this will break consumers of PETSc using the PETSc config params and (trying) to use mpi-cxx. However it will fix all consumers that do NOT use mpicxx (e.g. C applications). There is at least SLEPc matching that last category and none I know of matching the first. Hence me voting for this.

Better solution: Use that flag only for building PETSc (to be exact: the included/downloaded SuperLU) but not propagating it through PETSc to consumers. That would be correct. But I don't know of any possibility to do it besides building SuperLU via EC (cause PETSc-downloaded-deps to be a lie)

It is not a solution to fix consumers, because PETSc is build wrongly (has the mpicxx dependency which it should not) and may not even be possible.

Maybe patch the generated files and remove this flag after installation? How?

@boegel
Copy link
Copy Markdown
Member

boegel commented Jan 14, 2020

Patching the generated files that pass on -DOMPI_SKIP_MPICXX sounds reasonable.

Do you know if the SuperLU they pull in is a standard version, or a patched one?

If it's a standard version, I'd prefer taking the approach of providing a SuperLU built with EasyBuild...

@Flamefire
Copy link
Copy Markdown
Contributor Author

No idea, I guess standard. Problem is: It seems PETSc-downloaded-deps was specifically created NOT to use EC modules judging from the naming.

@boegel
Copy link
Copy Markdown
Member

boegel commented Jan 14, 2020

Indeed, but I think the main reason for that was two-fold: effort required, and the fact that some dependencies are patched.
We now have a good reason to flesh out SuperLU specifically (unless PETSc uses a patched version).

@Flamefire
Copy link
Copy Markdown
Contributor Author

I investigated again: It does not use patches as far as I can tell. However it is not SuperLU but SuperLU_DIST which causes the failure (as I've written above but confused myself due to not knowing there are multiple flavors). The Download and configuration process used by PETSc is easy though, so I guess I can come up with a suitable EB and EC (or maybe only EC for now and see if that works already). This could end up being quite some work though, so not sure if it is reasonable after all.

Comment thread easybuild/easyblocks/p/petsc.py Outdated
akesandgren
akesandgren previously approved these changes Mar 3, 2020
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Copy Markdown
Contributor

Was this everything needed to fix this part of the problem?
Have not been able to keep track of all the discussions going on the past weeks... only seen lots of stuff on petsc and other things and i'm probably mixing them up...

Due to SuperLU (automatically downloaded and build with PETSc) using mpicxx
the resulting libpetsc.so will have a dependency on C++ MPI symbols causing
failures in C applications linking against it even though those symbols are never used.

This removes the include to the mpi cxx extension inside openmpis mpi.h.
Downside is that all consumers of PETSc will have this define too as
PETSc saves its used command line flags.
@Flamefire
Copy link
Copy Markdown
Contributor Author

@boegel suggested to patch out these added flags so consumers can decide if they want MPICXX. I'll check how hard that would be

@Flamefire
Copy link
Copy Markdown
Contributor Author

@akesandgren I rebased to develop and added a commit which removes the flags from petscvariables (which AFAIK is the file used for packages using PETSc) so consumers can use MPICXX if the system / module is configured for it

--> Ready to go

@akesandgren
Copy link
Copy Markdown
Contributor

Trying to kick Hound

@akesandgren akesandgren closed this Mar 3, 2020
@akesandgren
Copy link
Copy Markdown
Contributor

Kicked

@akesandgren akesandgren reopened this Mar 3, 2020
@akesandgren
Copy link
Copy Markdown
Contributor

@Flamefire Please repush to kick Hound again.

@Flamefire Flamefire force-pushed the petsc_ignore_mpicxx branch from d8f2600 to ee8076f Compare March 4, 2020 08:06
@Flamefire
Copy link
Copy Markdown
Contributor Author

@akesandgren Done and passed

Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Copy Markdown
Contributor

Going in, thanks @Flamefire!

@akesandgren akesandgren merged commit 13c80ec into easybuilders:develop Mar 4, 2020
@Flamefire Flamefire deleted the petsc_ignore_mpicxx branch March 4, 2020 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants