Newton fail test fix #3787
Newton fail test fix #3787
Conversation
source/simulator/solver_schemes.cc
Outdated
| @@ -440,7 +441,7 @@ namespace aspect | |||
| // don't need to force assembly of the matrix. | |||
| if (stokes_matrix_depends_on_solution() | |||
| || | |||
| (nonlinear_iteration == 0 && boundary_velocity_manager.get_active_boundary_velocity_conditions().size() > 0)) | |||
| (nonlinear_iteration == 0 )) | |||
There was a problem hiding this comment.
Is this an accident? I think that the original condition should have had an || instead of && -- that strikes me as a bug in the original code. Or do I misunderstand the situation?
| // Test whether the rhs has dropped so much that we can assume that the iteration is done. | ||
| if (dcr.residual < dcr.residual_old * 1e-8) | ||
| { | ||
| return; |
There was a problem hiding this comment.
Could you output some status message here? Say
| return; | |
| pcout << " Nonlinear residual is small; skipping Stokes solve" << std::endl; | |
| return; |
There was a problem hiding this comment.
I don't think that your new message text reflects what is happening well. You are saying that "it has been very large", but that's not the point -- it's just been the regular nonlinear residual. The point is that it is vastly smaller this time around, and that that's likely because the model is (more or less) a linear model that is adequately solved in one iteration, and you're now in the next iteration. I think that my text reflected that better: it's about being small this time; it may or may not have been large last time around, we really neither know nor care.
There was a problem hiding this comment.
Related, what you print in parentheses is not actually the old residual that you reference when you say "has been very large"), but the ratio. It's confusing to see a statement "has been very large (1.234e-13)" I think :-)
There was a problem hiding this comment.
hmm, isn't the point that the reduction in the residual has been very large? Do you have a better way of phrasing it? Should I just state that the reduction in the residual has been very large, and/or do you want me to show a different number?
There was a problem hiding this comment.
I think I misread the text. I though you had said "Nonlinear residual has been very large", but you're saying "Nonlinear residual reduction has been very large. Yes, now it makes sense to me.
MFraters
force-pushed
the
newton_fail_test_fix
branch
from
577b32a
to
7fecb54
Compare
|
Thanks for the review, I think that I have addressed your comments. |
This currently failing test is a linear problem, so the second nonlinear iteration should do 0 iterations (and does when you use "single Advection, iterated Stokes") but it crashes with the Stokes solver not converging. It looks like we try to reduce the residual to 1e-20, which won't work. The test passes if you remove "set Linear solver tolerance".
MFraters
force-pushed
the
newton_fail_test_fix
branch
from
7fecb54
to
458c99a
Compare
|
I have changed the output a little bit to be a bit more descriptive and I updated the test results. |
MFraters
force-pushed
the
newton_fail_test_fix
branch
from
458c99a
to
0f21c59
Compare
MFraters
force-pushed
the
newton_fail_test_fix
branch
from
0f21c59
to
6053047
Compare
| // Test whether the rhs has dropped so much that we can assume that the iteration is done. | ||
| if (dcr.residual < dcr.residual_old * 1e-8) | ||
| { | ||
| return; |
There was a problem hiding this comment.
I don't think that your new message text reflects what is happening well. You are saying that "it has been very large", but that's not the point -- it's just been the regular nonlinear residual. The point is that it is vastly smaller this time around, and that that's likely because the model is (more or less) a linear model that is adequately solved in one iteration, and you're now in the next iteration. I think that my text reflected that better: it's about being small this time; it may or may not have been large last time around, we really neither know nor care.
| // Test whether the rhs has dropped so much that we can assume that the iteration is done. | ||
| if (dcr.residual < dcr.residual_old * 1e-8) | ||
| { | ||
| return; |
There was a problem hiding this comment.
Related, what you print in parentheses is not actually the old residual that you reference when you say "has been very large"), but the ratio. It's confusing to see a statement "has been very large (1.234e-13)" I think :-)
|
OK to merge if it tests ok. |
This is a proposed fix for the problem found in #3705. I made a new pull request out of it to allow me to easily change the code.
In short, this introduces a test of whether the rhs has dropped more than 1e-8 (I could make this a parameter if desired).
While fixing this, I noticed that to get it working I had to enable rebuilding of the Newton stokes matrix every 0th nonlinear iteration. It doesn't seem to change anything for the other tests (as far as I could see locally) because they already did that due to their boundary conditions.