Tupek/tr core mfem subspace - #1629
Conversation
…ions that we can use with our new matrix-free nonlinear solver.
…g forward. Try to simpify some of the testing and changes.
…mmon mfem functions, pull out more free functions.
|
|
||
| #pragma once | ||
|
|
||
| #include <cstddef> |
btalamini
left a comment
btalamini
left a comment
There was a problem hiding this comment.
I have a few questions, but they're small.
| int num_previous_steps = 2; | ||
|
|
||
| /// Relative CG forcing term for TrustRegion model solves. | ||
| double cg_forcing_rel = 1.2981521889723316e-05; |
There was a problem hiding this comment.
Could you give this a more meaningful name and explanation? I can suggest cg_residual_reduction_for_early_exit, but I'm open to better ideas.
| void checkProjectionInputs(const std::vector<const mfem::Vector*>& states, | ||
| const std::vector<const mfem::Vector*>& Astates, const mfem::Vector& b) | ||
| { | ||
| MFEM_VERIFY(states.size() == Astates.size(), |
There was a problem hiding this comment.
Should these be our own assertions, rather than MFEM ones?
| double dot(const mfem::Vector& a, const mfem::Vector& b) { return a * b; } | ||
|
|
||
| double norm(const mfem::Vector& x) { return x.Norml2(); } | ||
|
|
||
| double sumAbs(const mfem::Vector& x) | ||
| { | ||
| double total = 0.0; | ||
| for (int i = 0; i < x.Size(); ++i) { | ||
| total += std::abs(x[i]); | ||
| } | ||
| return total; | ||
| } |
There was a problem hiding this comment.
Your tests probably cover this, but just to be thorough: are these ok in parallel?
|
|
||
| size_t rootOnlyPrintLevel(const mfem::NewtonSolver& solver, size_t level) | ||
| { | ||
| #ifdef MFEM_USE_MPI |
There was a problem hiding this comment.
I'll need @white238 to confirm, but I think we always build with MPI and the guards are unnecessary. In fact, this function could possibly be reduced to a clear one-liner.
|
|
||
| std::vector<mfem::Vector> kept_columns; | ||
| for (int i = 0; i < evals.Size(); ++i) { | ||
| if (evals[i] > 1e-9 * trace_mag) { |
There was a problem hiding this comment.
Probably ok here, but hard-coded tolerances are a code smell. Want to make sure you're ok with this one.
ebchin
left a comment
ebchin
left a comment
There was a problem hiding this comment.
Overall, looks good. Mostly questions about testing.
|
|
||
| } // namespace | ||
|
|
||
| void TrustRegionSubspaceCache::prepare(const std::vector<const mfem::Vector*>& directions, |
There was a problem hiding this comment.
I had trouble finding this implementation since it's declaration is in trust_region_subspace_cache.hpp. Can you clarify the choices behind the file names?
| const mfem::Vector& b, int num_leftmost, MPI_Comm comm = MPI_COMM_WORLD); | ||
|
|
||
| /// @brief solves cached reduced trust-region problem for given trust-region radius | ||
| TrustRegionSubspaceResult solve(double delta) const; |
There was a problem hiding this comment.
Is there any testing for re-use of the cached values?
| } | ||
|
|
||
| /// apply trust region specific preconditioner | ||
| void precond(const mfem::Vector& x_, mfem::Vector& v_) const |
5 participants
The intent here is mostly to get away from requiring PETSc to use the 'subspace' option for the trust-region solver, and to have additional header files for the trust-region implementation so the equations_solver.cpp does not keep getting larger. There were some additional performance optimizations, such as reducing the total dot products and the number of parallel reductions, saving off subspace results when doing linesearching (as the subspace does not change), tuned some defaults based on a suite of benchmarks that I will add in a follow on PR, and some other small adjustments.