-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add support for J2 plasticity #243
Conversation
Great! Re where it goes: I'll read it through let you know if I have any thoughts/preferences. Re testing: This is mostly a note for myself. Verification Test 1 mentioned here might be of use, but I need to read through it in more detail. |
5db93ae
to
a8a806c
Compare
Analytical solution on sphere and cyclinder: Possible code comparison: |
a8a806c
to
6b4b9fa
Compare
9c3b793
to
a21c949
Compare
This PR should actually work if you add material in serial. I need to check that it works properly though. |
I think this is PR should be good enough for what Ayana needs (ie we can add material in serial but not use AMR). Of course, we still don't have good tests so that's pretty annoying. |
@Rombur, sounds good. I'll review and merge as soon as possible. The test case for Ayana will at least give us a sanity check until we have good tests in place. |
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.
This looks great. I have just a few minor suggestions for edits/additions of comments and changes to variable names.
source/MechanicalPhysics.cc
Outdated
{ | ||
// The deformation is plastic. We need to compute a new stress and | ||
// update the plastic internal variable and the back stress. | ||
double plastic_strain = |
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.
I'd suggest using plastic_strain_increment
instead of plastic_strain
since
double plastic_strain = | |
double plastic_strain_increment = |
source/MechanicalPhysics.cc
Outdated
double plastic_strain = | ||
(effective_stress_norm - _plastic_internal_variable[cell_id][q]) / | ||
(2. * mu + plastic_modulus); | ||
auto normalized_effective_stress = |
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.
Calling it a normalized effective stress is accurate, but physically it's a the direction of plastic flow. I suggested changing it to something like:
auto normalized_effective_stress = | |
auto plastic_flow_direction = |
to clarify how it should be interpreted.
source/MechanicalPhysics.cc
Outdated
else | ||
{ | ||
// The cell does not contain material or it is liquid. The displacement | ||
// is zero. |
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.
I think "The displacement is ignored." might be more accurate than "The displacement is zero.", since the displacement being zero could be interpreted as enforcing a rigid boundary for the solid material.
_dof_handler.active_cell_iterators() | | ||
dealii::IteratorFilters::ActiveFEIndexEqualTo(0, true)) | ||
{ | ||
// Compute the strain. We get the strain for all the quadrature points at |
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.
Consider putting here or elsewhere a brief mention of the reference. I know in my work it can be helpful to know where I got the formulation from a few months or years down the line.
// Compute the strain. We get the strain for all the quadrature points at | |
// Formulation based on the combined isotropic-kinematic hardening | |
// model for J2 plasticity in Chapter 3 of R. Borja, Plasticity: Modeling | |
// and Computation, Springer-Verlag, 2013. | |
// DOI: 10.1007/978-3-642-38547-6 | |
// Compute the strain. We get the strain for all the quadrature points at |
@Rombur There was a repeated phrase in one of the comments (I think probably due to how I structured the "suggested edit"). I went ahead and fixed that directly. Once the CI passes I'll merge this in. |
1a43a4d
to
acfb480
Compare
There was a failure (see here). I have seen this a couple of times. Usually re-running fixes the issue. I don't know where the problem comes from. I have run the test in valgrind and everything looks fine. |
The CI passed, so we can merge this. @stvdwtt where do you think the new code should go? MechanicalPhysics, MechanicalOperator, or a new Operator? I can move the code in a different PR if we think it would be better somewhere else. |
Merging it now. I think it works well in MechanicalPhysics, at least for now. If we get to a point where we have a more complicated model or options between models, it might make sense to move it to a new operator. But I don't have a good enough sense of where we're going with this next to justify a particular structure. |
This is still a work in progress. The first two commits could be moved to their own PRs but they are literally one line each so I don't think it matters.
Things left to do:
MechanicalPhysics
but we could put it inMechanicalOperator
. My reasoning is that, the new code does not need "solving" but I don't know where is the right place.