Skip to content
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

Add MUMPS package (and some dependencies) #246

Merged
merged 5 commits into from
Jul 29, 2021
Merged

Conversation

kinnewig
Copy link
Contributor

At the moment MUMPS is only available via PETSc, when deal.II was installed through candi.

Since I am using MUMPS via Trilions, I added one package for MUMPS and linked it into PETSc and Trilinos. I also added a package for ScaLAPACK since it is used by PETSc and MUMPS (otherwise, PETSc and MUMPS would each install their own version of ScaLAPACK and we would end up with two different installations of ScaLAPACK).

Is this package for MUMPS of interest for candi?

Comment on lines 91 to 93
if [ ! -z "${PARMETIS_DIR}" ]; then
CONFOPTS="${CONFOPTS} --download-scalapack=1"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check for parmetis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this check for ParMETIS only because in the PETSc package it checks if ParMETIS is present before installing MUMPS and ScaLAPACK.
See: petsc.package#L76-L87

I have not tested whether this check is necessary, I just assumed that it is necessary because it was already there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay I see, in the original there are the downloads for mumps and scalapack only if parmetis is present without explaining it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyhow, this looks strange and should be tested if this makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will test, if ParMETIS is necessary for MUMPS and ScaLAPACK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like ParMATIS is not necessary for MUMPS and ScaLAPACK, therefore I removed the check.

Comment on lines 49 to 57
if [ ! -z "${BLAS_DIR}" ]; then
CONFOPTS="${CONFOPTS} \
-D pc_blas_LIBRARY_DIRS=${BLAS_DIR}/lib "
fi

if [ ! -z "${BLACS_DIR}" ]; then
CONFOPTS="${CONFOPTS} \
-D MKL_INCLUDE_DIRS=${BLACS_DIR}"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be a problem if both, blas and mkl blacs, are given?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not quite sure.
But to be on the safe side, we can set either BLAS only or MKL BLACS only, depending on whether this is an MKL build or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took a closer look at how to properly link MUMPS to intel MKL and adapted these lines accordingly.

deal.II-toolchain/packages/mumps.package Show resolved Hide resolved

if [ ! -z "${BLACS_DIR}" ]; then
CONFOPTS="${CONFOPTS} \
-D MKL_INCLUDE_DIRS=${BLACS_DIR}"
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if we have the standard blacs from netlib and not the mkl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good remark!
The installer will not find the BLACS from netlib.

However, it is sufficient when either BLACS or BLAS is present.

Comment on lines 101 to 103
if [ ! -z "${PARMETIS_DIR}" ]; then
CONFOPTS="${CONFOPTS} --download-mumps=1"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check for parmetis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same answer as above: I have not tested whether this check is necessary, I just assumed that it is necessary because it was already there.

@@ -104,7 +121,7 @@ package_specific_setup () {

package_specific_register () {
export PETSC_DIR=${INSTALL_PATH}
if [ ! -z "${PARMETIS_DIR}" ]; then
if [ ! -z "${PARMETIS_DIR}" ] && [ -z "${SCALAPACK_DIR}" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check for parmetis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again the same answer as above: I have not tested whether this check is necessary, I just assumed that it is necessary because it was already there.

deal.II-toolchain/packages/petsc.package Show resolved Hide resolved
fi

# Set Fortran flags
FORTRAN_MAJOR_VERSION=$(echo __GNUC__ | ${FC} -E -xc - | tail -n 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

this works only if FC is given

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a test if FC is given.

But what can we do, when FC is not given? We still need to check the compiler version, since we need to set the Fortran_FLAGS correspondingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, than you can't do much, you can try finding gfortran as it is done in candi.sh for tools like wget.
But if FC is however not set, this may end in a strange error

Copy link
Contributor

Choose a reason for hiding this comment

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

simply put if [[ -n ${FC} ]] around this statement, this should be good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a test if FC is given.

deal.II-toolchain/packages/scalapack.package Show resolved Hide resolved

package_specific_register () {
export SCALAPACK_DIR=${INSTALL_PATH}
export BLACS_DIR=${INSTALL_PATH}/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

is blacs always installed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the README of ScaLAPACK:

New in version 2.0:

  • The BLACS is now part of ScaLAPACK, and is compiled into the ScaLAPACK
    library. It is no longer necessary to link against BLACS libraries.

Therefore I assume it is always installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, maybe put a comment somewhere such that it is clear that scalapack installs blacs.

@koecher
Copy link
Contributor

koecher commented Jul 19, 2021

At the moment MUMPS is only available via PETSc, when deal.II was installed through candi.

Since I am using MUMPS via Trilions, I added one package for MUMPS and linked it into PETSc and Trilinos. I also added a package for ScaLAPACK since it is used by PETSc and MUMPS (otherwise, PETSc and MUMPS would each install their own version of ScaLAPACK and we would end up with two different installations of ScaLAPACK).

Is this package for MUMPS of interest for candi?

I think it can be useful.

@ghost
Copy link

ghost commented Jul 19, 2021

Is this package for MUMPS of interest for candi?

I think it can be useful.

If I remember correctly the trilinos package may have also an interface to MUMPS, which may be interesting to have.

@kinnewig
Copy link
Contributor Author

I have only now been able to continue working on the pull request.

Nevertheless I checked in the meantime if ParMETIS is necessary to install MUMPS and ScaLAPACK in PETSc. Therefore I installed MUMPS and Scalapack via PETSc (without ParMETIS) and run some tests. As far as I can tell, ParMETIS is not necessary for MUMPS and ScaLAPACK to work with PETSc. Therefore I removed the check if ParMETIS is available.

If I remember correctly the trilinos package may have also an interface to MUMPS, which may be interesting to have.

Yes, Trilinos has an interface to MUMPS and I already linked MUMPS into Trilinos, since I am already using MUMPS via Trilinos in my code.

I also took a closer look at the installation of MUMPS with intel MKL and adapted the package accordingly.

@kinnewig
Copy link
Contributor Author

On ubuntu 20.04 the check failed because the package libxnvctrl0 could not be installed.
This seems to be a problem with the test on Ubuntu 20.04. I just tested the latest version which passed all tests last time and this time the test on ubuntu 20.04 also failed.

@koecher
Copy link
Contributor

koecher commented Jul 28, 2021

On ubuntu 20.04 the check failed because the package libxnvctrl0 could not be installed.

This seems to be a problem with the test on Ubuntu 20.04. I just tested the latest version which passed all tests last time and this time the test on ubuntu 20.04 also failed.

yes that is unrelated.

Copy link
Contributor

@koecher koecher left a comment

Choose a reason for hiding this comment

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

Thank you for that work and the requested changes.

@koecher koecher merged commit 4e48480 into dealii:master Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants