Skip to content
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 visco plastic floating point exception #2235

Merged
merged 1 commit into from May 9, 2018

Conversation

gassmoeller
Copy link
Member

@gassmoeller gassmoeller commented May 9, 2018

The visco plastic material model throws floating point exceptions when used with strain weakening, because it assumes to always get a strain rate, which is not the case. This was not covered by the existing test, because the test only solves the first timestep (for which strain weakening is explicitly disabled). The fix is simple, and might be important enough to be cherry picked to the release branch. But I will leave that decision up to @naliboff and @tjhei.

@gassmoeller gassmoeller requested a review from naliboff May 9, 2018 20:07
@gassmoeller gassmoeller added this to the 2.0 milestone May 9, 2018
@naliboff
Copy link
Contributor

naliboff commented May 9, 2018

@gassmoeller - Thanks for catching this and I'm surprised that none of us had run into that yet. In any case, the test should have certainly gone to at least one time step in the first place. The fix looks good and this should be ready to merge as soon as the tester is done.

I'll leave it up to @tjhei for whether or not this is incorporated into the release branch. It is an important fix, but the error does not produce any incorrect values.

Copy link
Contributor

@jdannberg jdannberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your fix makes sense, but it actually opens up the question if the visco plastic material model works with strain weakening as intended (as it seems there was no test for that).

The reason I am saying that is that there are places in the code where we assume that the material model inputs object does not need the strain rate because it does not compute the viscosity. Here, we need the strain rate to compute reaction terms. So how sure are we that the material model inputs have the strain rate in all places where this term needs to be computed? I am not saying that this is wrong, I just want to make sure that we have some kind of benchmark case for shear weakening.

@naliboff
Copy link
Contributor

naliboff commented May 9, 2018

@jdannberg - I originally checked manually to make sure that the friction and cohesion values were reduced correctly as a function of the accumulated strain. I have no idea why I did not do at least one time step to confirm this in the actual test.

If we want to have a test that confirms the values are correct, there is an option to output the friction and cohesion values through AdditionalMaterialModelOutputs. however, I'm not sure what the best method would be to get them in human readable form.

@jdannberg
Copy link
Contributor

@naliboff I was more worried about the accumulated strain being computed correctly (specifically the version that only uses the second invariant, which is the option that caused the problem here). It would be good to have a test case where we know how the finite strain should evolve over time (and therefore how the friction and cohesion values would change) and compare to that.

We have test cases for the finite strain in the benchmarks/finite_strain folder, so that could be a good starting point.

I'm not sure what the best method would be to get them in human readable form

There is the gnuplot output format (instead of vtk) that produces numbers that can be compared in the tests (you just need to add the gnuplot output file as one of the test outputs, and make sure it's not too big).

@jdannberg jdannberg merged commit cf95884 into geodynamics:master May 9, 2018
@naliboff
Copy link
Contributor

naliboff commented May 9, 2018

@jdannberg - Gotcha and I agree it would be good to have tests specifically for looking at the finite strain. We can use the tests in benchmarks/finite_strain for the cases with the full finite strain tensor and use a model with a fixed strain rate for the finite strain invariant. I'll put these tests together tonight or tomorrow morning.

@jdannberg
Copy link
Contributor

Thank you!

@gassmoeller gassmoeller deleted the fix_visco_plastic_fpe branch May 9, 2018 22:06
@bangerth
Copy link
Contributor

I think the question is where these reaction terms feed into? Are they used for a compositional model that is then used again in the material model?

@jdannberg
Copy link
Contributor

In this model, the reaction terms represent the finite strain occurring in a given time step and are used to compute the accumulated finite strain everywhere in the model, which then feeds back into the material model (the higher the accumulated strain, the lower the viscosity).
So the only relevant place where we have to make sure we compute the term correctly is the assembly of the composition advection equation, and that gets the full material model inputs including the strain rate. So in principle, this part should now work as intended. But I still think we should have a test case to see if the second invariant of the finite strain is tracked correctly, and if it influences the material properties correctly (in a similar way as we have these tests for the full finite strain tensor, which are the tests I mentioned above).

@bangerth
Copy link
Contributor

bangerth commented May 10, 2018 via email

@tjhei
Copy link
Member

tjhei commented May 10, 2018

This looks harmless enough to include in 2.0...

@tjhei
Copy link
Member

tjhei commented May 10, 2018

I cherry-picked it for 2.0 (without a PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants