You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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,
the clear function is for clearing data changed by solve functions but not options set by users to the solver in my opinion;
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;
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.
The text was updated successfully, but these errors were encountered:
YaqiWang
added a commit
to YaqiWang/libmesh
that referenced
this issue
Jul 21, 2022
@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.
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.
This:
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 callEigenSystem::solve ()
(that calls one of the solve functions inSlepcEigenSolver
multiple times). But because thatclear
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 havingset_eigensolver_type
function.I propose to remove that line because,
solve
functions but not options set by users to the solver in my opinion;_position_of_spectrum
, etc., which should be cleared as well if we think we should reset the solver type inclear
;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.
The text was updated successfully, but these errors were encountered: