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

[EigenSolvers] Add FEAST with MKL support to EigenSolversApplication #6482

Merged
merged 50 commits into from
Apr 7, 2020

Conversation

qaumann
Copy link
Contributor

@qaumann qaumann commented Feb 28, 2020

If the EigenSolversApplication is compiled with MKL support, FEAST can now also be included to solve generalized eigenvalue problems. With this FEAST interface, it is possible to find either the lowest or highest eigenvalues for real symmetric problems or some eigenvalues inside a specified range for real and complex symmetric problems. The support can also be extended to general matrices or polynomial problems, if required.

The interface to FEAST in EigenSolversApplication differs from the one in ExternalSolversApplication in the following points:

  • Here, FEAST 4.0 is used. With the new version, it is possible to look for the highest / lowest eigenvalues in a specified range
  • The builtin MKL support is used for the matrix computations, so the complicated reverse communication interface is not required

There is one small change in the core, which makes the Solve function for complex eigensystems available to Python.

Tests are provided and the readme is updated. I tested it on Linux and will also try to run it on Windows. This is still a draft, so tell me what you think. I am happy for suggestions.

@oberbichler
Copy link
Contributor

For the eigenvalue solver I have also tried FEAST. It turned out that (at least the version in MKL) is very sensitive to the choice of parameters. Especially the the search range was problematic. Do you also have this problem?

@qaumann
Copy link
Contributor Author

qaumann commented Mar 3, 2020

I also experienced this, but would not call it a problem. I like the feature, that you can limit your eigenvales to a specific range very much and this is also important for some methods I am using. The only parameter that can really influence your results is the subspace size, but there is a functionality in FEAST which estimates a suitable size. It would be no big problem to expose this also in Kratos, e.g. as a process or a InitializeSolutionStep.

In the new FEAST 4, you can, however, also look for extremal eigenvalues, e.g. the lowest in your defined search interval, which would start at zero for structural problems. This is the same functionality as the EigensystemSolver, if I am not mistaken.

Regarding Windows compilation, I managed to compile FEAST, but linking it with all required Fortran libraries is something that needs further investigation and currently I do not have enough time for this. I hope this is no showstopper here.

I also discusses with @philbucher if we should include FEAST's source code in the repository, like it is done in ExternalSolversApplication. Then the compilation of the library can be switched on and off directly from the configure file.

@philbucher
Copy link
Member

To me it would be ok adding the feast code into the repo
We could do so in a separate PR, since it also would involve the Comittee

How big is it?
The license is MIT right?

@oberbichler
Copy link
Contributor

It was a problem because it was not possible to find a general setup to find the smallest eigenvalues for our benchmarks. The new functions are definitely an enrichment.

About the fortran-stuff: AFAIK it is not so easy to integrate fortran in Windows with the default toolchain. But I think we can start the FEAST integration as an exclusive Linux feature.

BTW: Is there any connection to Eigen?

@qaumann
Copy link
Contributor Author

qaumann commented Mar 4, 2020

The only connection is MKL. And that I thought, EigenSolversApplication is an application for solving the eigenvalue problem :)

@oberbichler
Copy link
Contributor

Was not the intention of the application but with the MKL dependency it makes sense to but it here.

@qaumann
Copy link
Contributor Author

qaumann commented Mar 13, 2020

@philbucher I added the FEAST library to external_libraries. It weighs 2.5mb and is licensed under BSD. Compilation can now be included by -DUSE_FEAST4 on Linux. After some days of trying, I gave up on the Windows compilation, as the linking from the Fortran library to MKL is not too trivial. If anyone has experience with this, I would love to learn more about it.

@qaumann qaumann marked this pull request as ready for review March 16, 2020 09:13
@philbucher
Copy link
Member

I created #6645 since I didn't want to pollute this branch

I noticed that the tests test_complex_symmetric_gev and test_real_symmetric_gev take multiple seconds on my machine. Same for you?

Other than that this is ready, I will discuss it in the committee

@qaumann
Copy link
Contributor Author

qaumann commented Mar 31, 2020

I noticed that the tests test_complex_symmetric_gev and test_real_symmetric_gev take multiple seconds on my machine. Same for you?

Other than that this is ready, I will discuss it in the committee

Thank you very much for the review.

On my machine the tests run for around 0.7 seconds each (in release). This is because I have to copy each column of the eigenvector matrix to a new vector, as the ublas space (at least its Python interface) does not support matrix-matrix-multiplication. If it takes too long, I can adapt the tests.

@philbucher
Copy link
Member

On my machine the tests run for around 0.7 seconds each (in release). This is because I have to copy each column of the eigenvector matrix to a new vector, as the ublas space (at least its Python interface) does not support matrix-matrix-multiplication. If it takes too long, I can adapt the tests.

hm if it ok in the CI then I guess it is fine
If you really want you could try to expose this and check but not necessary, up to you

…ast-improvements

Improvements for [EigenSolvers] Add FEAST with MKL support to EigenSolversApplication #6482
@qaumann
Copy link
Contributor Author

qaumann commented Mar 31, 2020

If you really want you could try to expose this and check but not necessary, up to you

I would rather not in the scope of this PR. Another problem is, that it's a multiplication of sparse and dense matrices. I will change the matrix used in the test, if the CI complains.+

And regarding enable_if, I would stick to using it in CreateFEAST, if there is nothing severe against it.

@philbucher
Copy link
Member

@KratosMultiphysics/technical-committee this PR adds the latest version of FEAST in the EigenSolversApplication.
An older version of FEAST is also in the ExternalSolversApplication, hence I think adding this should not be a problem for anyone
As @qaumann mentioned above the added files sizes are 2.5MB which I think is ok

If you agree then we don't have to use meeting-time for this ;)

@philbucher
Copy link
Member

you need to merge master, there are some conflicts

@qaumann
Copy link
Contributor Author

qaumann commented Apr 3, 2020

the conflict should be resolved

@RiccardoRossi
Copy link
Member

@KratosMultiphysics/technical-committee proposes the following
1 - accept this PR (and try to remove the old feast from the ExternalSolversApplication)
2 - in a following PR change the name EigenSolversApplication->LinearSolversApplication
3 - add, in a followup PR the (optional) compilation of SuperLU to the LinearSolversApplication
4 - move to legacy the ExternalSolversApplication

IMPORTANT: by default, everything has to compile without BLAS (mkl, or any blas) and without fortran. This means that in the tests we CANNOT use anything depending on those features

Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

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

@qaumann I think this is ready, please feel free to merge
I made a small comment but this can be a future PR

It would be also nice if you could do the following two things:

  • Port the tests in StructuralMechanics that use the FEAST of the ExternalSolversApp to use the solvers of the EigenSolversApp?
  • If that is done remove Feast from the ExternalSolversApp as requested by @KratosMultiphysics/technical-committee

Nice work overall and thanks for the patience 👍

template<>
Parameters SettingsHelper<double>::GetDefaultParameters()
{
return Parameters(R"({
Copy link
Member

Choose a reason for hiding this comment

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

@qaumann I suggest you implement / clean up in a follow up PR

@qaumann
Copy link
Contributor Author

qaumann commented Apr 7, 2020

@philbucher Thanks again for the review and the comments, I will address them now.

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.

4 participants