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
Fix some small Newton related problems #2100
Conversation
Did you want to include all of these other things into this pull request? |
not all of them, only the add derivatives pull reqest. I can see if I can clean it up a bit |
c956037
to
5f3e6db
Compare
Hm, then I don't quite understand. Does this patch rely on functionality that is still pending in other PRs? Or why is it so much? |
yes, or part of my testing relies on the added utilities function in the #2030, but the order it is shown in here look quite random. There are the three important commits:
I can also just push them without the commits from #2030 |
@bangerth, I tested turning the n_basis_for_tensor function into just 1's, like you suggested, but it is still different. The values look something like this: (nonlinear channel flow with drucker_prager_compositions)
(spiegelman with drucker_prager_compositions)
|
else | ||
viscosity = ref_visc; | ||
|
||
viscosity =ref_visc*viscosity/(ref_visc+viscosity); |
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.
Found the problem. Forgot that this term was also present in the Spiegelman paper, and should also present in the derivative. This explains why the analytical derivative didn't converge well, while the finite difference did.
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.
OK, great. So everything now agrees?
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.
It only agrees when I use
0 1/2
1/2 0
And only when some parts of the strain-rate are not zero, because when they are the analytical derivatives in that part of the tensor becomes zero, and the finite differences still has a value other then zero.
I now think that the correct form for these units tensors is actually
That's because this is a tensor that has norm 1. |
Please rebase and let's see how this works then. |
16cb232
to
a2829ca
Compare
So I managed to fix two problems. The first one has to do with the fact that the derivative of the regularization we computed actually needed the unregularized viscosity to compute the regularized derivative. The second problem was that I never added the analytical pressure derivative. Everthing seems to work fine, but I cheated a bit on the analytical pressure derivative, because I removed a |
ow, I forgot to mention, the analytical and the finite difference derivative are now exactly the same, except for the off diagonal terms, which are very slightly off, but it doesn't seem to hinder convergence:
|
On 02/21/2018 12:20 AM, MFraters wrote:
|analytical = 9.11386e+37, fd = 9.11386e+37
analytical = -9.11377e+37, fd = -9.11377e+37
analytical = -1.08329e+32, fd = -1.11412e+32|
The difference is about 0.03e32=3e30 out of a total of almost 1e38. This
difference is zero.
|
9606f36
to
6c451aa
Compare
ac4b5fc
to
f2abaf3
Compare
@bangerth, to summarize, there are two things we need to decide.
|
About 1: Do you mean this line?
This looks wrong. Both of the definitions we discuss in the paper on p. 6 use About 2: Is |
about 1: It comes I think from that people often in rheologies a term about 2: the material model |
On 03/01/2018 01:07 PM, MFraters wrote:
about 1: It comes I think from that people often in rheologies a term |2.0 *
edot_ii| appears somewhere, see for example the drucker prager (line 106) and
the visco plastic material model (line 246 and 291). But I am totally fine
with removing it here, it would make more sense to me.
I think I'm still not quite clear about how the factor of 2 got there. If the
formula reproduces a material model that has been used in a different paper,
then we should have the same factors that they have as well. If we are
discussing a material model without reference to other works, then the factor
of two seems wrong.
(I can't look up right now where the lines you reference are -- I assume you
are referencing lines in the paper -- because I don't have an internet
connection: I'm somewhere over the Aleutians in the middle of the North Pacific.)
about 2: the material model |drucker_prager.cc| was already there before the
paper, and I think it is used in the crustal cookbooks. You are right that non
of the benchmarks we show in the paper use pure drucker prager, but we discuss
pure ducker prager in appendix B2. That is why I ask whether we should put it
somewhere in aspect, so that people can for themself see that alpha is always
a half if you implement it in a pure way.
I see. Yes, I think that would actually be quite nice. It reminds me that we
should get better at providing material models *as building blocks* for other
material models. For example, the regularized Drucker Prager model should not
reimplement things, but just *build* on a pure DP model.
That is something to fix for a later time, but if you were willing to add such
a pure DP model, then that would be nice!
|
about 1: I talked to @cedrict and we looked it up in the place where the benchmark originally comes from, which is the book of Taras Gerya (equation 16.4). In there the 2 is present, so I think we should keep it. about 2: Should I add a pure Drucker Prager in this or an other pull request? |
On 03/07/2018 09:42 AM, MFraters wrote:
about 1: I talked to @cedrict <https://github.com/cedrict> and we looked it up
in the place where the benchmark originally comes from, which is the book of
Taras Gerya (equation 16.4). In there the 2 is present, so I think we should
keep it.
OK, let's keep it then.
about 2: Should I add a pure Drucker Prager in this or an other pull request?
Separate is always easier.
|
I will add a reference to the comments |
9dae3f4
to
6ed6f40
Compare
To summarize, the wasn't really an spd problem in the end, we just didn't account for the regularization. In the mean time we found three other issues, which we solved here.
|
There are some problems with the derivatives. This pull request intents to fix them, but it is currently a work in progress, as discussed with @bangerth.