-
Notifications
You must be signed in to change notification settings - Fork 238
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
Update spiegelman newton benchmark #6159
Update spiegelman newton benchmark #6159
Conversation
2384f1d
to
561ac80
Compare
561ac80
to
d29f915
Compare
Ok, @MFraters I think this is ready for a review and I could address most of the things I didnt understand originally. This PR does 3 things:
Just as a reminder, these are the original results from the paper: And these are the results I could produce with this PR: |
3b8c363
to
0357203
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gassmoeller for working to fix the convergence and cleaning up the benchmark code. In hindsight we probably should have split up #5580 in three different pull request, so that all individual changes to the solver could be reviewed and tested separately.
It is nice to see that the both the unstabilized and stabilized versions are now generally faster and have a more stable convergence rate than in the paper, and in some cases can converge, where the paper couldn't (although for the hardest case, it is the other way around, but that was just one sub-case which may just have gotten lucky).
I will wait with merging for a day in case @bangerth or @YiminJin still want to take a look at it, but I think it is good to merge.
So this started out as a follow-up to #6135 to update the Spiegelman 2016 benchmark for the Newton solver. It ended up being a lengthy dive into our version history and I think I need outside input on how to put this together (@MFraters can we speak some time?). Before the wall of data here are my conclusions from the work:
metabash.sh
andbash.sh
is very confusing (I simplified it here). It should be easy to reproduce the figure from the paper to check this benchmark for correctness. Also we should include the standardinput.prm
benchmark case as a test.So here is the output data I have, while trying to reproduce Fig. 4 of Fraters et al. 2019:
Original Figure 4 from paper:
My version run with ASPECT 2.5 (AMG). I can not guarantee this uses identical parameters to the paper, because the figure and scripts included in ASPECT where clearly different from the original paper (I tried to reproduce according to the description of the paper). This is already different from the paper, but still close:
This is the most relevant comparison to ASPECT 3.0 (AMG). You can see how the SPD stabilized models (dots) converge better than above, but the non-stabilized versions (lines) are much worse:
Here are the results of some other things I tried:
Results with ASPECT 3.0 (GMG), for comparison with AMG above. Some models converge better, some worse:
Results with ASPECT 2.6.pre (GMG). This version was ASPECT right before #6135 was merged to tease apart the influence of #6135. There are some changes, but in both directions (worse/better). On average I would say the behavior is similar:
As mentioned above, I am not sure what conclusion to draw from all of this. I tried to look for problems/changes in the code that caused the different convergence behavior, but apart from the obvious (different SPD stabilization factor) I have trouble understanding the following:
Why is the DC Picard convergence in all models now worse than the Fixed Point Picard convergence? This was not the case in the original paper, but was already present in ASPECT 2.5. Maybe the residual is now computed differently than back then?this was a bug in my run scriptWhat was the change the destroyed the quadratic convergence for (some) of the unstabilized models? It must have happened between 2.5 and 3.0, independent of linear solver (/tolerance), independent of stabilization. Some change in the Jacobian matrix?see discussion below and [WIP] Revert one change of PR #5580 to improve Newton solver convergence #6160Some of the ASPECT 3.0 models show a sudden increase in nonlinear residual right when switching from defect-correction solver to Newton solver that was not present in 2.5 or earlier models. I can prevent this increase by allowing for more line-search iterations (=not moving in the direction of increasing residual), but this leads some models to never converge.this must have been introduced in Fixup newton solver and elastic rheology #5580 and is only present in the stabilized models. since the stabilized models are now better than before once they do converge, it is likely an acceptable changeI think it would be great if we could figure out some of these questions.