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

BGRa creep model #2167

Merged
merged 22 commits into from Aug 1, 2018

Conversation

Projects
None yet
6 participants
@wenqing
Copy link
Member

wenqing commented Jul 20, 2018

This PR adds the BGRa creep model with two tests.

Note: it contains the content of #2164 temporarily.

@wenqing

This comment has been minimized.

Copy link
Member Author

wenqing commented Jul 20, 2018

Please review only after #2164 is merged and this PR is rebased.

@OlafKolditz

This comment has been minimized.

Copy link
Member

OlafKolditz commented Jul 20, 2018

@wenqing wenqing force-pushed the wenqing:creepBGRa branch from f0234b7 to 0c5a8f3 Jul 20, 2018

@chleh

chleh approved these changes Jul 20, 2018

Copy link
Collaborator

chleh left a comment

Looks good. Only minor comments. Didn't check the test results and the correctness of the formulas.


std::unique_ptr<
typename MechanicsBase<DisplacementDim>::MaterialStateVariables>
createMaterialStateVariables() override

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

Does this forwarded method have to be overridden explicitly?

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

Removed overridden.

class CreepBGRa final : public LinearElasticIsotropic<DisplacementDim>
{
public:
static int const KelvinVectorSize =

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

Probably all these Kelvin variables and typedefs are already inherited from the base class, I assume.

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

After removed it, I got compilation error of "KelvinVectorSize is not defined".

This comment has been minimized.

@chleh

chleh Jul 23, 2018

Collaborator

OK. Thanks for checking. Maybe then each usage would have to be prefixed with the base class LinearElasticIsotropic<DisplacementDim>::KelvinVectorSize. But that's too ugly.

const double A, const double n, const double sigma0, const double Q)
: LinearElasticIsotropic<DisplacementDim>(std::move(mp)),
_nonlinear_solver_parameters(std::move(nonlinear_solver_parameters)),
_A(A),

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

In the past underscoreCAPITALLETTER members led to problems.

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

_A was removed in the next commits. With the change, it is used in the newly introduced member _coef.

There is another member _Q and it is changed to _q.

private:
NumLib::NewtonRaphsonSolverParameters const _nonlinear_solver_parameters;

const double _A;

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

Docu missing?

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

This member is gone.

NumLib::NewtonRaphson<decltype(linear_solver), JacobianMatrix,
decltype(update_jacobian), ResidualVectorType,
decltype(update_residual),
decltype(update_solution)>(

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

Just a minor general comment: If the type list of NumLib::NewtonRaphson was reordered,
one could write the above lines as
NumLib::NewtonRaphson<JacobianMatrix, ResidualVectorType>(...)
due to type inference.

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

I tried but failed. I will ask you for how to later.

This comment has been minimized.

@chleh

chleh Jul 23, 2018

Collaborator

Then also the type list of NumLib::NewtonRaphson in NumLib has to be reordered accordingly. It's only a very minor issue, so IMHO it your code can stay as it is.

double const norm_s_n1 = std::sqrt(2.0 * Invariants::J2(s_n1));
pow_norm_s_n1_n_minus_one_2b_G =

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

Oh no, now update_jacobian is not a pure function anymore, but has side effects. 😄
Maybe there's a performance gain from that. But if there are errors or refactorings, the code is harder to understand.
I'd maybe add comments // side effect to the two assignments above.

This comment has been minimized.

@wenqing

wenqing Jul 30, 2018

Author Member

Yes, the only reason is the performance. I have added a little long comment above the definitions of the two variables to explain the reason.

@@ -20,6 +22,13 @@ double Invariants<KelvinVectorSize>::equivalentStress(
return std::sqrt(3 * J2(deviatoric_v));
}

template <int KelvinVectorSize>
double Invariants<KelvinVectorSize>::Norm(

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

That's the Frobenius norm of the tensor represented by deviatoric_v, isn't it?
I'd call the function FrobeniusNorm(). And it's also valid for general Kelvin vectors, isn't it?

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

Yes, it it. Changed accordingly.

// where J_(sigma) is the Jacobian of the last local Newton-Raphson
// iteration, which is already LU decomposed.
KelvinMatrix tangentStiffness =
(*success_iterations == 0) ? C : linear_solver.solve(C);

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

can success_iterations == 0 ever happen?

This comment has been minimized.

@endJunction

endJunction Jul 20, 2018

Member

Yes. If I remember correctly, in the first time step only, somehow the residual is 0 leading to 0 iterations...

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

Yes, it happens when ||r^0||=0. For example with isotropic initial stress , in the case of the first time step and the first global Newton step, we have ||s||=0 and ||d\sigma| =0 (an elasticity status), meaning ||r||=0.

This comment has been minimized.

@chleh

chleh Jul 23, 2018

Collaborator

Thanks for the explanation

where
$\mathbf{C}:= \lambda \mathcal{J} + 2G \mathbf I \otimes \mathbf I $
with $\mathcal{J}$ the forth order identity, $\mathbf I$ the second order identity,
$\lambda$ the Lame constant, $G$ the shear module, and $\otimes$ the tensor

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

Lamé constant, shear modulus.

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

Corrected.

$\lambda$ the Lame constant, $G$ the shear module, and $\otimes$ the tensor
product notation.

is a forth order tensor. Substituting equation and the expression of $C$

This comment has been minimized.

@chleh

chleh Jul 20, 2018

Collaborator

fourth

This comment has been minimized.

@wenqing

wenqing Jul 20, 2018

Author Member

Corrected.

@chleh Thanks for your comments.

@wenqing

This comment has been minimized.

Copy link
Member Author

wenqing commented Jul 20, 2018

jenkins did not run

@bilke

This comment has been minimized.

Copy link
Member

bilke commented Jul 20, 2018

@wenqing I restarted the Jenkins job.

@wenqing wenqing force-pushed the wenqing:creepBGRa branch from 0c5a8f3 to f3b7c60 Jul 20, 2018

private:
NumLib::NewtonRaphsonSolverParameters const _nonlinear_solver_parameters;

const double _n; /// parameter determined by experiments.

This comment has been minimized.

@nagelt

nagelt Jul 23, 2018

Member

I'd specify this to: "creep rate exponent n"

This comment has been minimized.

@wenqing

wenqing Jul 30, 2018

Author Member

Changed.

return ConstitutiveModel::Invalid;
}

/// Get temperature related coefficient for the global assembly if there is

This comment has been minimized.

@nagelt

nagelt Jul 23, 2018

Member

does this relate to the Arrhenius contribution? If so, I'd name it that. "Temperature related coefficient" is a little vague ... maybe "ArrheniusTemperatureCoefficient"

This comment has been minimized.

@chleh

chleh Jul 23, 2018

Collaborator

For me "Arrhenius" always translates to exp(-E / RT). Maybe "Arrhenius" too narrow.

This comment has been minimized.

@wenqing

wenqing Jul 30, 2018

Author Member

I am considering that there may be other temperature models, e.g. temperature dependent cam-clay like model, which has another type of temperature term. Therefore, I would like to have a general name.

@nagelt

nagelt approved these changes Jul 23, 2018

Copy link
Member

nagelt left a comment

I only had minor comments ... looks good! Thanks, Wenqing!

@wenqing wenqing force-pushed the wenqing:creepBGRa branch from f3b7c60 to 898b8bb Jul 30, 2018

pow_norm_s_n1_n_minus_one_2b_G *
(deviatoric_matrix + ((_n - 1) / (norm_s_n1 * norm_s_n1)) *
s_n1 * s_n1.transpose());
jacobian = KelvinMatrix::Identity() +

This comment has been minimized.

@wenqing

wenqing Jul 30, 2018

Author Member

Only formatted.

@wenqing wenqing force-pushed the wenqing:creepBGRa branch from 47820c4 to 4c1df12 Jul 30, 2018

@wenqing wenqing force-pushed the wenqing:creepBGRa branch from 1fa015a to 0f9bfee Jul 30, 2018

@bilke

bilke approved these changes Jul 31, 2018

Copy link
Member

bilke left a comment

Had just a look at the docs: 👍

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Aug 1, 2018

@wenqing If there are no outstanding changes, then merge it.

@wenqing wenqing merged commit b936997 into ufz:master Aug 1, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.