Add strain weakening option to the visco_plastic material model #1192

naliboff · 2016-08-29T16:49:55Z

Implementation of a strain weakening option to the visco_plastic material model. Strain weakening is calculated via finite strain, which may be calculated through particles or compositional fields through the finite_strain plugin.

For either of these options, the material model assumes (and states in the documentation) that the fields tracking each finite strain component must be listed prior to any additional fields. This allows removing the contribution of the finite strain fields from the volume fraction of each element. Is there a better way to do this? I can think of a few other methods, but am interested to hear new ideas on this point.

gassmoeller · 2016-08-31T18:08:05Z

/run-tests

gassmoeller · 2016-08-31T18:13:19Z

source/material_model/visco_plastic.cc

+          // Strain weakening
+          double strain_ii = 0;
+          if (use_strain_weakening == true)
+


I think you are missing some curly braces here.

Ah, it might work like this as well, but I would prefer if you would do everything up to line 220 only if use_strain_weakening is set to true. This way it is clearer that all of that only matters for strain weakening. And the declaration of strain_ii might go into that if-loop as well if you do not need it later on.

naliboff · 2016-09-06T16:33:16Z

@gassmoeller - thanks for the thorough review, going through point by point now.

naliboff · 2016-10-21T22:18:43Z

@gassmoeller - I think this is good to go. I am holding off on changing how the finite strain fields are handles in the cell averaging, as the whole material model is going to be reworked in the not too distant future.

gassmoeller

Thanks, this is much better. There are two minor issues left, and the question about the default value for use_strain_weakening. Usually we try to make sure parameter files run with a newer ASPECT version give the same (or very similar) results to the original version, therefore I would prefer the default to be 'false'. What is you opinion?

gassmoeller · 2016-10-24T16:49:44Z

source/material_model/visco_plastic.cc

+              phi = phi + ( phi - phi * friction_strain_weakening_factors[j] ) * strain_fraction;
+
+              //
+              if ( (this->get_timestep_number() == 6) && (strain_ii > 0.01) && (j==4))


I think this might be debug output? Please remove that.

gassmoeller · 2016-10-24T16:51:53Z

source/material_model/visco_plastic.cc

+                             "If only one value is given, then all use the same value.  Units: $1 / K$");
+
+          // Strain weakening parameters
+          prm.declare_entry ("Use strain weakening", "true",


Do you prefer "true" to be the new default value? This will change user models that were done with this material model retroactively, usually we try to avoid such changes, unless they seem to be better in any case (but in this case it will change results unpredictably). I would prefer to have a "false" default. This would also remove the changes to the visco_plastic test output below. What do you think?

gassmoeller · 2016-10-24T16:52:28Z

source/material_model/visco_plastic.cc

+                                   "The user has the option to linearly reduce the cohesion and "
+                                   "internal friction angle as a function of accumulated finite strain. "
+                                   "Finite strain may be calculated through tracers (implemented by Rene Gassmoeller) "
+                                   "or the compositional field finite strain plugin (implemented Juliane Dannberg). "


implemented by

gassmoeller · 2016-10-24T16:56:31Z

After making the changes you will also have to replace the test output by the official output from Timo's tester.

naliboff · 2016-10-25T16:51:02Z

Thanks Rene. I will update once the tester has finished.

naliboff · 2016-11-01T23:04:13Z

Hmm, the tester is still failing on one of the visco_plastic_yield tests. However, looking at the diff file it is not clear to me what exactly the tester is having a problem with. I'll try re-running the test and see if the tester gives the same errors.

naliboff · 2016-11-02T00:21:15Z

Ok, the tester seems to be working except for the gccpetsc tests on 8.4.1.b. It is failing on the boundary_composition/ascii_data test. Is there anything else I can do on my end or this is a separate tester issue?

bangerth · 2016-11-04T21:21:55Z

I can't seem to match anything in this patch with the error in question. Can you rebase to current master and force push your branch again? That should restart the tester on the current state.

naliboff · 2016-11-09T00:21:56Z

@bangerth - I just rebased with the current master and forced pushed to my branch. The tester appears to have restarted and hopefully this fixes and clarifies the issue.

bangerth · 2016-11-09T02:27:07Z

There is still a problem with that test, though I can't say what exactly is going wrong there :-(

naliboff · 2016-11-09T03:28:29Z

Odd, it was a different test failing last time. I'll re run the test on my end, update the output and see if the tester at least gives a diff file.

naliboff · 2016-11-09T05:34:09Z

The last test failing was almost certainly due to not having a new parameter for updating ghost particles this is required with active tracers. I should have rerun the test to make sure everything was working after the rebase.

naliboff · 2016-11-09T18:45:06Z

All tests are passing now.

bangerth · 2016-11-09T22:51:33Z

I'll let @gassmoeller see whether he was happy with this. In the meantime, can you squash all of your commits into one or a smaller number?

naliboff · 2016-11-09T23:19:32Z

@bangerth - the pull request is now squashed into 3 commits.

naliboff · 2016-11-10T00:13:58Z

@gassmoeller - tests are passing, but it seems with the rebase/squash I accidentally picked up an additional commit? Is this ok to merge or do I need to remove commit e882953 from my history?
Edit - I went ahead and just removed it, apologies if this causes any issues.

gassmoeller · 2016-11-10T02:09:55Z

I am happy with the changes. Thanks for staying with us through the revisions!
Do you want to add an entry to doc/changes.h? This will give you credit for the work in the next release letter. If so: Could you also remove the accidental changes in the output of the visco_plastic and visco_plastic_yield test results? They have not changed in any significant way, but you overwrote the timing result with one of your machine (that will not be checked by the tester anyway). Ready to merge once you have decided about the entry to changes.h.

naliboff · 2016-11-10T18:29:17Z

@gassmoeller - Of course and thanks for all the improvements with the review! I will indeed add something to changes.h. For the two tests you mention, exactly what part of the screen-output files needs to be changed? If it is the timing section starting with the wall clock speed, should I be replacing with the timing output from the tester?

gassmoeller · 2016-11-11T01:23:36Z

For the two tests I mentioned, only the timing output at the end of the screen output was changed by your commit (I suppose you have run the tests on your machine and updated the output files although nothing had changed). But these timings are irrelevant for the tester. You can see the changes here: https://github.com/geodynamics/aspect/pull/1192/files#diff-da161af4088a5c66c7d20f9ae6594b18

The two things you could do:

Remove the lines containing the numbers (take a look at other test output how that might look like). You can also achieve this by running the test with ctest and copy the resulting screen-output.notime file over the existing one.
Revert the files to their state on master. There are git command to achieve this, but the simplest thing to do might be to switch to master, copy the two files to backup locations. Switch back to your branch and move the backup files over the existing ones (effectively replacing the file version on your branch with the ones from master).

naliboff · 2016-11-14T22:29:53Z

Test output updated and added text to changes.h. There is a conflict with changes.h, though. Should I update this a rebase on my end, or can github resolve the merge conflict?

bangerth · 2016-11-14T23:37:11Z

You'll need to rebase by hand, unfortunately, to resolve the merge conflict.

naliboff · 2016-11-15T00:38:26Z

Fixed, should be good to go 👍

gassmoeller reviewed Aug 31, 2016
View reviewed changes

gassmoeller requested changes Oct 24, 2016

View reviewed changes

naliboff force-pushed the strain_weakening branch from d4b5c0a to eeb6e0d Compare November 9, 2016 00:18

naliboff force-pushed the strain_weakening branch from 057ac71 to b2ad54a Compare November 9, 2016 23:16

naliboff force-pushed the strain_weakening branch from b2ad54a to 52eac73 Compare November 10, 2016 01:11

gassmoeller approved these changes Nov 10, 2016

View reviewed changes

naliboff and others added 4 commits November 14, 2016 16:28

Add strain weakening option to the visco_plastic material model

d4c029f

Strain invariant is now calculated through existing dealii functions

8a76832

Update visco_plastic tests

9504b09

Update changes.h and test output

0a20531

naliboff force-pushed the strain_weakening branch from a90fd08 to 0a20531 Compare November 15, 2016 00:36

gassmoeller merged commit c7f291d into geodynamics:master Nov 16, 2016

naliboff deleted the strain_weakening branch January 27, 2017 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add strain weakening option to the visco_plastic material model #1192

Add strain weakening option to the visco_plastic material model #1192

naliboff commented Aug 29, 2016

gassmoeller commented Aug 31, 2016

gassmoeller Aug 31, 2016

gassmoeller Aug 31, 2016

naliboff commented Sep 6, 2016

naliboff commented Oct 21, 2016

gassmoeller left a comment

gassmoeller Oct 24, 2016

gassmoeller Oct 24, 2016

gassmoeller Oct 24, 2016

gassmoeller commented Oct 24, 2016

naliboff commented Oct 25, 2016

naliboff commented Nov 1, 2016

naliboff commented Nov 2, 2016

bangerth commented Nov 4, 2016

naliboff commented Nov 9, 2016

bangerth commented Nov 9, 2016

naliboff commented Nov 9, 2016

naliboff commented Nov 9, 2016

naliboff commented Nov 9, 2016

bangerth commented Nov 9, 2016

naliboff commented Nov 9, 2016

naliboff commented Nov 10, 2016 •

edited

gassmoeller commented Nov 10, 2016

naliboff commented Nov 10, 2016

gassmoeller commented Nov 11, 2016

naliboff commented Nov 14, 2016

bangerth commented Nov 14, 2016

naliboff commented Nov 15, 2016

Add strain weakening option to the visco_plastic material model #1192

Add strain weakening option to the visco_plastic material model #1192

Conversation

naliboff commented Aug 29, 2016

gassmoeller commented Aug 31, 2016

gassmoeller Aug 31, 2016

Choose a reason for hiding this comment

gassmoeller Aug 31, 2016

Choose a reason for hiding this comment

naliboff commented Sep 6, 2016

naliboff commented Oct 21, 2016

gassmoeller left a comment

Choose a reason for hiding this comment

gassmoeller Oct 24, 2016

Choose a reason for hiding this comment

gassmoeller Oct 24, 2016

Choose a reason for hiding this comment

gassmoeller Oct 24, 2016

Choose a reason for hiding this comment

gassmoeller commented Oct 24, 2016

naliboff commented Oct 25, 2016

naliboff commented Nov 1, 2016

naliboff commented Nov 2, 2016

bangerth commented Nov 4, 2016

naliboff commented Nov 9, 2016

bangerth commented Nov 9, 2016

naliboff commented Nov 9, 2016

naliboff commented Nov 9, 2016

naliboff commented Nov 9, 2016

bangerth commented Nov 9, 2016

naliboff commented Nov 9, 2016

naliboff commented Nov 10, 2016 • edited

gassmoeller commented Nov 10, 2016

naliboff commented Nov 10, 2016

gassmoeller commented Nov 11, 2016

naliboff commented Nov 14, 2016

bangerth commented Nov 14, 2016

naliboff commented Nov 15, 2016

naliboff commented Nov 10, 2016 •

edited