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

support scaling and GMRES in Eigen linear solvers #1509

Merged
merged 6 commits into from
Nov 7, 2016

Conversation

norihiro-w
Copy link
Collaborator

This PR introduces

  • add OGS_USE_EIGEN_UNSUPPORTED CMake option (default is ON) to support unsupported modules in Eigen. If this option is ON, the followings become available:
  • support scaling of a linear system before solve. <scaling>1</scaling> should be specified under <eigen> tag in a config file.
  • support GMRES solver type

@@ -12,6 +12,8 @@
#include <logog/include/logog.hpp>

#ifdef USE_EIGEN_UNSUPPORTED
#include <iostream> // to fix compiling errors in Eigen
Copy link
Member

Choose a reason for hiding this comment

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

Please add a version of Eigen where this is needed in the comment. Could help later to eliminate unnecessary includes.

Copy link
Collaborator Author

@norihiro-w norihiro-w Nov 3, 2016

Choose a reason for hiding this comment

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

The error happens in unsupported/Eigen/src/IterativeSolvers/ConstrainedConjGrad.h, which is included in unsupported/Eigen/IterativeSolvers.h. I checked the history of the file and the header file is not included since 2009. maybe I should change it to include only GMRES. ✅

@endJunction
Copy link
Member

Can you drop a pointer to the "scaling" documentation, my google does not produce adequate results.

@norihiro-w
Copy link
Collaborator Author

see

@norihiro-w
Copy link
Collaborator Author

@bilke It seems Eigen package in conan does not include "unsupported" directory. Could you please add the directory to the package?

@endJunction
Copy link
Member

Thanks for the pointers.

Copy link
Collaborator

@chleh chleh left a comment

Choose a reason for hiding this comment

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

Looks good in general

#ifdef USE_EIGEN_UNSUPPORTED
if (auto scaling =
//! \ogs_file_param{linear_solver__eigen__scaling}
ptSolver->getConfigParameterOptional<int>("scaling")) {
Copy link
Collaborator

@chleh chleh Nov 3, 2016

Choose a reason for hiding this comment

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

Why don't you use bool? Please use bool. ✅

@@ -29,6 +32,10 @@ EigenOption::SolverType EigenOption::getSolverType(const std::string &solver_nam
return SolverType::BiCGSTAB;
if (solver_name == "SparseLU")
return SolverType::SparseLU;
#ifdef USE_EIGEN_UNSUPPORTED
if (solver_name == "GMRES")
return SolverType::GMRES;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious: Is there a specific reason why you don't use LIS' Gmres?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, there is no reason and I prefer LIS. However currently those who download executables from the OGS website can use only Eigen solvers and they should be able to choose GMRES for unsymmetric matrices. BiCGSTab cannot be used for all the problems as it can break down.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, thanks.

@ogsbot
Copy link
Member

ogsbot commented Nov 4, 2016

Jenkins: OGS-6/Linux-PRs-dynamic failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs-dynamic/1585/

@TomFischer
Copy link
Member

Looks good. When Eigen package is 'fixed': ⏩

Copy link
Member

@wenqing wenqing left a comment

Choose a reason for hiding this comment

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

👍

@bilke
Copy link
Member

bilke commented Nov 7, 2016

@norihiro-w Eigen unsupported-directory should be in the package now.

@norihiro-w
Copy link
Collaborator Author

@bilke thanks! how can I restart the Jenkins job?

@bilke
Copy link
Member

bilke commented Nov 7, 2016

@norihiro-w I restarted it (currently there is no possibility for you to restart, I will add that later on) and it ran successfully.

@norihiro-w norihiro-w merged commit 38247b9 into ufz:master Nov 7, 2016
@norihiro-w norihiro-w deleted the eigen-unsupported branch November 7, 2016 13:45
@norihiro-w
Copy link
Collaborator Author

thanks!

@ogsbot
Copy link
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

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.

7 participants