-
Notifications
You must be signed in to change notification settings - Fork 239
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
[WIP] Revert one change of PR #5580 to improve Newton solver convergence #6160
[WIP] Revert one change of PR #5580 to improve Newton solver convergence #6160
Conversation
32f0537
to
0c785e9
Compare
@gassmoeller Sorry for the late reply. The reason that I use deviatoric strain rate in the Jacobian matrix in PR #5580 is that there were only few rheologies ( I noticed that in the Spiegelman fail test, the parameter The modifications I made in PR #5580 was some sort of hard coding. We may have to find some way to tackle this problem. |
@YiminJin Thanks for the feedback! I agree with your statement that using the deviatoric strain rate provides much better convergence rate for models that use the stabilization, this is also what I observed in my tests in #6159 and which is the reason I kept using the deviatoric strain rate if the PD stabilization is applied. However, I am still in favor of including my modification in this PR. I have the the following two reasons (tell me if I am wrong here):
Would you agree with this? Let me know in case I got something wrong, I only really started looking into this in the past few weeks. |
Thank you for the detailed reply. With the help of your explanation, I realized that I made two mistakes in my previous comment:
I have tested your modifications with the modified continental extension cookbook in Issue #6046, and found that the convergence rate was much better than the main branch when no stabilization is applied (the main branch takes 101 iterations with a lot of line search steps to converge to 10-7 in the first timestep, while the present PR takes only 52 iterations and no line search step). |
Thanks for checking! Glad to know you come to the same conclusion. I will close this PR then, as the change I proposed here was merged as part of #6159 already.
Yes that would be good to understand better. When I tried using the full strain rate with the PD stabilization I got warnings that the matrix was no longer positive definite (these checks/warnings are only enabled in debug mode).
Also a good question, but I suspect it is related to material model averaging. One could for example run the Spiegelman benchmark set of #6159 with AMG and harmonic material model averaging and compare that against GMG with harmonic material model averaging. If they more or less agree, then the difference is likely just that GMG requires the averaging, while AMG does not. |
This is a result of #6159. I would like to discuss reverting this change that happened originally in #5580. The difference between the
strain_rate
andeffective_strain_rate
is for most models (that dont use elasticity) thateffective_strain_rate
is simply the deviatoric part of the strain rate. I do not understand why we use the deviatoric strain rate here (it will be used to assemble the Newton matrix, i.e. the Jacobian). Maybe I just didnt follow the discussion in #5580 closely enough. From my understanding this should be the full strain rate, and at least the benchmark in #6159 massively benefits from my change here:These are the relevant results from the benchmark in #6159 pre #5580:
These are the relevant results post #5580. You can see you the stabilized version / dots converge faster, but the unstabilized version / lines are much worse:
Now these are the relevant results post #5580 but with the change I propose in this PR. The unstabilized models are reverted to their original (better) convergence, and even the stabilized models gain another small boost:
I think this change may be responsible for some of the changes in #6159 that I couldnt explain. I will have to rerun more benchmarks, but in the meantime @YiminJin and @MFraters could you remind me why we did this change originally?