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

SlepcEigenSolver clear function resetting the default solver type which may prevents calling solve function properly #3341

Open
YaqiWang opened this issue Jul 21, 2022 · 2 comments · May be fixed by #3342

Comments

@YaqiWang
Copy link
Contributor

This:

      // SLEPc default eigenproblem solver
      this->_eigen_solver_type = KRYLOVSCHUR;

in void SlepcEigenSolver<T>::clear () function is giving me a trouble.

I've called void set_eigensolver_type (const EigenSolverType est) to set the desired the solver type and need to call EigenSystem::solve () (that calls one of the solve functions in SlepcEigenSolver multiple times). But because that clear function resets the default solver type and is called by all the solve functions, I cannot make the second solve call to use the same non-default solver type I set previously. Because of this, I have to use a different way to set the solver type with SLEPc command-line option -eps_type, which defeats the purpose of having set_eigensolver_type function.

I propose to remove that line because,

  1. the clear function is for clearing data changed by solve functions but not options set by users to the solver in my opinion;
  2. there are other settings in the solver like _position_of_spectrum, etc., which should be cleared as well if we think we should reset the solver type in clear;
  3. we could add a new function like clearSolveData and called by solve functions, but I think it is unnecessary. Users can reset the solver type manually if they want.

I've tried removing that line and ran all MOOSE tests. All tests past. I will push up a PR to let civet check if there are any implications.

YaqiWang added a commit to YaqiWang/libmesh that referenced this issue Jul 21, 2022
YaqiWang added a commit to YaqiWang/libmesh that referenced this issue Jul 21, 2022
YaqiWang added a commit to YaqiWang/libmesh that referenced this issue Jul 21, 2022
@YaqiWang
Copy link
Contributor Author

@roystgnr suggested to remove the call to clear() in solve functions. We just do not know whether the call was added there on purpose. @jwpeterson or @fdkong possibly have other ideas.

@jwpeterson
Copy link
Member

The call to clear() was most likely originally added because we weren't sure about the implications of reusing the underlying SLEPc objects for multiple solves. So it's possible that it's not inherently necessary, but that something else in the meantime has come to rely on the clear() being there. We won't really know until we test it.

I disagree with your point 1. above since the original purpose of the Solver class' clear() function was always to return the object to a "just-constructed" state as much as possible, including resetting options to their default values. I don't know if there was ever a lot of use of clear() for that purpose, though. What we actually used clear() for was calling it from destructors, but that practice largely became unnecessary with the creation of std::unique_ptr.

This PR reminds me that we kind of lost track of #3208 as well.

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 a pull request may close this issue.

2 participants