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

TrilinosWrappers::SolverDirect: adjust interface #16727

Merged
merged 1 commit into from Mar 13, 2024

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Mar 7, 2024

... to be able to switch between TrilinosWrappers::PreconditionILU, TrilinosWrappers::PreconditionAMG, TrilinosWrappers::SolverDirect.

The changes are:

  • derive from Subscriptor
  • allow to pass initialize() an AdditionalData object
  • introduce vmult functions
  • make SolverControl optional

/**
* Constructor. Creates the solver without solver control object.
*/
SolverDirect(const AdditionalData &data = AdditionalData());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
SolverDirect(const AdditionalData &data = AdditionalData());
explicit SolverDirect(const AdditionalData &data = AdditionalData());

* factorization for it with the package chosen from the additional
* data structure. Note that there is no need for a preconditioner
* here and solve() is not called.
*/
Copy link
Member

Choose a reason for hiding this comment

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

@p data replaces the data stored in this instance.

@@ -559,6 +573,23 @@ namespace TrilinosWrappers
solve(dealii::LinearAlgebra::distributed::Vector<double> &x,
const dealii::LinearAlgebra::distributed::Vector<double> &b);

/**
* Solve the linear system <tt>Ax=b</tt> based on the
* package set in initialize(). Note the matrix is not refactorized during
Copy link
Member

Choose a reason for hiding this comment

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

or in the constructor, right?

@bangerth
Copy link
Member

bangerth commented Mar 7, 2024

Are you deriving from Subscriptor so that you have a base class that you can do a dynamic_cast from? That seems like a questionable thing to do given that the interfaces of the various classes are incompatible. Typically, one would have an interface class where derived classes implement the interface. Would that work?

If that doesn't work, why don't you store these objects via std::variant? This avoids the dynamic_cast from an otherwise rather arbitrary base class.

@peterrum
Copy link
Member Author

peterrum commented Mar 9, 2024

@tjhei I have addressed the comments.

Are you deriving from Subscriptor so that you have a base class that you can do a dynamic_cast from? That seems like a questionable thing to do given that the interfaces of the various classes are incompatible. Typically, one would have an interface class where derived classes implement the interface. Would that work?
If that doesn't work, why don't you store these objects via std::variant? This avoids the dynamic_cast from an otherwise rather arbitrary base class.

@bangerth No. The multigrid classes store SmartPointer instances which require that the classes inherit from Subscriptor.

Copy link
Member

@kronbichler kronbichler 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 to me apart from a minor typo.

include/deal.II/lac/trilinos_solver.h Outdated Show resolved Hide resolved
@kronbichler kronbichler merged commit 43a7737 into dealii:master Mar 13, 2024
16 checks passed
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.

None yet

4 participants