Roy/saddle point preconditioning - #1598
Conversation
|
@lc-hubcast run pipeline |
|
I've started a new pipeline for you! |
|
@lc-hubcast approve |
|
To approve the sync of this PR, please use the GitHub review comment feature to submit an approval. This ensures the approval is tied to a specific commit to avoid unintended syncing of malicious commits. |
|
|
||
| HYPRE_Int GetBoomerAMGNumFunctions(const mfem::HypreBoomerAMG& amg) | ||
| { | ||
| HYPRE_Solver h = amg; // mfem::HypreBoomerAMG has operator HYPRE_Solver() |
There was a problem hiding this comment.
is this a deep copy or shallow copy?
| mfem::DenseMatrix inv_; | ||
| }; | ||
|
|
||
| class GuessSensitiveIterativeSolver : public mfem::IterativeSolver { |
There was a problem hiding this comment.
This seems like a odd name for this class. NoOpIterativeSolver, or something like that? Also, comment to explain why it returns nan if iterative_mode.
There was a problem hiding this comment.
What is the goal for moving SolverWithPreconditioner out of equation_solver.cpp?
There was a problem hiding this comment.
So it can easily be included elsewhere
There was a problem hiding this comment.
There should be a base class BlockPreconditioner that inherits mfem::Solver and owns member functions like numSubSolvers and subSolvers (essentially common member functions shared by different types of block preconditioners). It should also own shared member variables like block_offsets_. Then the specific type of block preconditioner will inherit this base class. For example class BlockDiagonalPreconditioner : public BlockPreconditioner
There was a problem hiding this comment.
@lihanyu97 That's a good point. I added a base class in my latest commit. It reduces repetition.
There was a problem hiding this comment.
The changes looks good. It looks like you have to document the member functions and variables in the base class in order to pass the test. Other than that, the PR is good.
| struct BlockSolverAccessor { | ||
| int n = 0; | ||
| mfem::Solver* (*get)(void*, int) = nullptr; | ||
| void* self = nullptr; | ||
| }; | ||
|
|
||
| const BlockSolverAccessor acc = [&]() { | ||
| if (auto* p = dynamic_cast<smith::BlockDiagonalPreconditioner*>(mfem_solver)) { | ||
| return BlockSolverAccessor{p->numSubSolvers(), static_cast<mfem::Solver* (*)(void*, int)>([](void* self, int i) { | ||
| return static_cast<smith::BlockDiagonalPreconditioner*>(self)->subSolver(i); | ||
| }), | ||
| p}; | ||
| } | ||
| if (auto* p = dynamic_cast<smith::BlockTriangularPreconditioner*>(mfem_solver)) { | ||
| return BlockSolverAccessor{p->numSubSolvers(), static_cast<mfem::Solver* (*)(void*, int)>([](void* self, int i) { | ||
| return static_cast<smith::BlockTriangularPreconditioner*>(self)->subSolver(i); | ||
| }), | ||
| p}; | ||
| } | ||
| if (auto* p = dynamic_cast<smith::BlockSchurPreconditioner*>(mfem_solver)) { | ||
| return BlockSolverAccessor{p->numSubSolvers(), static_cast<mfem::Solver* (*)(void*, int)>([](void* self, int i) { | ||
| return static_cast<smith::BlockSchurPreconditioner*>(self)->subSolver(i); | ||
| }), | ||
| p}; | ||
| } | ||
| return BlockSolverAccessor{}; | ||
| }(); |
There was a problem hiding this comment.
This would not be necessary once the base block preconditioner class is defined.
|
|
||
| if (acc.get) { | ||
| for (int i = 0; i < acc.n && static_cast<size_t>(i) < us.size(); ++i) { | ||
| configure_amg(acc.get(acc.self, i), us[static_cast<size_t>(i)]->space().GetVDim()); |
There was a problem hiding this comment.
This would become
| configure_amg(acc.get(acc.self, i), us[static_cast<size_t>(i)]->space().GetVDim()); | |
| configure_amg(block_preconditioner.subSolver(i), us[static_cast<size_t>(i)]->space().GetVDim()); |
5 participants
To be merged after tupek/system_solver #1556