-
Notifications
You must be signed in to change notification settings - Fork 326
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
Support to use external arpack libraries #554
Conversation
IF(NOT EXTERNAL_ARPACK) | ||
ADD_SUBDIRECTORY(arpack) | ||
IF(MPI_FOUND) | ||
ADD_SUBDIRECTORY(parpack) | ||
ENDIF() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be possible to use an external ARPACK in combination with the PARPACK that is bundled with Elmer?
What do you think about this change?
IF(NOT EXTERNAL_ARPACK) | |
ADD_SUBDIRECTORY(arpack) | |
IF(MPI_FOUND) | |
ADD_SUBDIRECTORY(parpack) | |
ENDIF() | |
IF(NOT ARPACK_FOUND) | |
ADD_SUBDIRECTORY(arpack) | |
ENDIF() | |
IF(MPI_FOUND AND NOT PARPACK_FOUND) | |
ADD_SUBDIRECTORY(parpack) | |
ENDIF() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, depending on the components' search results, you can have a mixed configuration (arpack: external and parpack: internal, or vice-versa). Your proposed change would act as a failsafe mechanism if something went wrong. However, I think it could defeat the original idea of explicitly using external libraries.
Plus, some platforms provide separate packages for arpack and parpack. So if the user forgets to install both, this omission could go unnoticed if they ignore the cmake's output.
Perhaps the solution to have a mixed configuration is to split the logic of FindARPACK.cmake into two scripts (FindARPACK.cmake and FindPARPACK.cmake) and add a new variable (EXTERNAL_PARPACK::BOOL).
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separating the Find* modules for the two libraries and having dedicated EXTERNAL_*
options sounds like a good idea to me. 👍
I haven't tested this yet. But for the case that the external ARPACK doesn't provide a CMake module that imports the respective targets, shouldn't it be necessary to create the CMake import targets |
Also, use the provided information by the config script to setup some variables.
We have a local change in PARPACK. If I rememeber correctly, parack extracted a wrong set
of eigenvectors from the computed Lanczoz results. The change was made before we switched
from SVN to GIT, so hard time to find the log....
This of course doesn't prevent you from giving an option to link to external libraries as long as the
original libraries are the default option.
Cheers!
|
parack extracted a wrong set of eigenvectors from the computed Lanczoz results
... and only for complex valued input...
|
Yep, you are right. I have added a new commit taking into consideration your comments. |
It looks like this was merged before the CMake module for PARPACK has been split from the CMake module for ARPACK. @ealbiter: Could you please suggest the necessary changes in a follow-up PR? |
Ok, sorry I was a little hasty! As the ARPACK and PARPACK have so far always been married, I guess this does not break anything that has been? |
No, the default is still to use the bundled implementations. The ARPACK package in MSYS2 doesn't contain a parallel version of ARPACK. So, it won't be possible to use the ARPACK package from MSYS2 but still use the bundled PARPACK. |
Porpouse
With this pull request, you can add support for using external arpack libraries provided by the system. Nowadays, several platforms provide precompiled packages of the arpack library based on the arpack-ng project, which can be used to compile elmerfem.
Currently, the compiled arpack libraries are installed in ELMER_INSTALL_LIB_DIR, which prevents the installation of the system-provided arpack package if this path is set to the system lib path. Consequently, other software that depends on this package can not be installed (e.g., octave, igraph).
Proposed changes
A new cmake variable (EXTERNAL_ARPACK::BOOL) can be used to employ the internal arpack library or the system-provided one. By default, the internal arpack libraries will be used. The user optionally can set variables to define the path to the libraries' location and include directory (ARPACK_LIBRARIES, ARPACK_INCLUDE_DIR, and if MPI is used PARPACK_LIBRARIES, PARPACK_INCLUDE_DIR). If the user does not specify these variables, cmake will try to find the libraries using the FindARPACK.cmake script.