-
Notifications
You must be signed in to change notification settings - Fork 239
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
Conversation
@@ -12,6 +12,8 @@ | |||
#include <logog/include/logog.hpp> | |||
|
|||
#ifdef USE_EIGEN_UNSUPPORTED | |||
#include <iostream> // to fix compiling errors in Eigen |
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.
Please add a version of Eigen where this is needed in the comment. Could help later to eliminate unnecessary includes.
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.
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. ✅
Can you drop a pointer to the "scaling" documentation, my google does not produce adequate results. |
see
|
@bilke It seems Eigen package in conan does not include "unsupported" directory. Could you please add the directory to the package? |
Thanks for the pointers. |
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.
Looks good in general
#ifdef USE_EIGEN_UNSUPPORTED | ||
if (auto scaling = | ||
//! \ogs_file_param{linear_solver__eigen__scaling} | ||
ptSolver->getConfigParameterOptional<int>("scaling")) { |
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.
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; |
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.
Just curious: Is there a specific reason why you don't use LIS' Gmres?
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.
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.
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.
OK, thanks.
Jenkins: OGS-6/Linux-PRs-dynamic failed: https://svn.ufz.de:8443/job/OGS-6/job/Linux-PRs-dynamic/1585/ |
Looks good. When Eigen package is 'fixed': ⏩ |
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.
👍
07fa422
to
186f2ce
Compare
@norihiro-w Eigen unsupported-directory should be in the package now. |
@bilke thanks! how can I restart the Jenkins job? |
@norihiro-w I restarted it (currently there is no possibility for you to restart, I will add that later on) and it ran successfully. |
thanks! |
OpenGeoSys development has been moved to GitLab. |
This PR introduces
OGS_USE_EIGEN_UNSUPPORTED
CMake option (default is ON) to support unsupported modules in Eigen. If this option is ON, the followings become available:<scaling>1</scaling>
should be specified under<eigen>
tag in a config file.