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
Newton merge 3, part 2: Newton drucker prager material #1684
Newton merge 3, part 2: Newton drucker prager material #1684
Conversation
Can anyone with better knowledge of material models look at this? @gassmoeller , @anne-glerum ? |
/run-tests |
aa24b5e
to
44e1246
Compare
const double pressure=std::max(in.pressure[i],0.0); | ||
|
||
// calculate effective viscosity | ||
if (in.strain_rate.size()) |
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.
add ">0"
|
||
const SymmetricTensor<2,dim> strain_rate_deviator = deviator(in.strain_rate[i]); | ||
|
||
const double edot_ii_strict = (&(this->get_simulator()) == NULL |
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.
please add the long comment that you deleted in this place.
out.viscosities[i] = effective_viscosity; | ||
} | ||
|
||
Assert(dealii::numbers::is_finite(out.viscosities[i]),ExcMessage ("Error: Averaged viscosity is not finite.")); |
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.
space after comma, maybe break line
|
||
} | ||
} | ||
out.densities[i] = reference_rho * (1 - thermal_expansivity * (temperature - reference_T)); |
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.
1.0
out.thermal_conductivities[i] = thermal_conductivities; | ||
// Compressibility at the given positions. | ||
// The compressibility is given as | ||
// $\frac 1\rho \frac{\partial\rho}{\partial p}$. |
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.
comment wrong
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.
what should it be?
tests/drucker_prager_derivatives.cc
Outdated
|
||
if (Error) | ||
{ | ||
std::cout << "Some parts of the test where not succesfull." << std::endl; |
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.
successful
@@ -0,0 +1 @@ | |||
set Additional shared libraries = tests/libdrucker_prager_derivatives.so |
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.
add a comment please
oneone 3: Finite difference = 0. Analytical derivative = -0 | ||
oneone 4: Finite difference = -5.03316e+25. Analytical derivative = -0 | ||
Error: The derivative of the viscosity to the strain rate is too different from the analitical value. | ||
Some parts of the test where not succesfull. |
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.
so the test fails?
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.
ah, right. I had written a bit about that in the big pull request, but forgot to copy it to here to discuss.
I had written something on this in a email to wolfgang a long time ago:
In short the problem is that for all common cases I found that the finite difference and the analytical solution are exactly the same. But with a strain rate in which the diagonal entries are the same the analytical solution gives -0 and the finite difference gives results on the order of the other strain rates (~-1e+20).
Here is the relevant code (There is also a minimum to edot_ii in the real code.):
const double edot_ii = 2 * std::sqrt(0.5*deviator(in.strain_rate[i])*deviator(in.strain_rate[i]));
const double stress_exponent_inv = (1./stress_exponent[c]);
composition_viscosities[c] = std::pow(prefactor[c],-stress_exponent_inv) * std::pow(edot_ii,stress_exponent_inv-1);
composition_viscosities_derivatives[c] = 2 * (stress_exponent_inv-1) * composition_viscosities[c] * (1/(edot_ii*edot_ii)) * deviator(in.strain_rate[i]);
I think this happens for the following reason: In the analytical equation, the deviator make the diagonal then zero, and the analytical solution then also ends up with zero entries on the diagonal. The finite difference method first gives the original strain rate tensor to the material model, and the deviator makes the diagonal zero. But then the finite difference method perturbs only one of the diagonal entries, which make the diagonal entries not equal any more and as a consequence the diagonal of the deviator will not contain zero entries any more, and I think this lead to a viscosity change on the order of what normally happens.
Thoughts?
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.
Looks ok except for some style issues. Thanks for unifying the functions. How rigorously did you test that the new viscosity computation is identical to the old one? What about corner cases (like epsilon=0)?
out.viscosities[i] = effective_viscosity; | ||
} | ||
|
||
Assert(dealii::numbers::is_finite(out.viscosities[i]),ExcMessage ("Error: Averaged viscosity is not finite.")); |
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.
Please break this line
for (int y = 0; y < dim; y++) | ||
Assert(dealii::numbers::is_finite(derivatives->viscosity_derivative_wrt_strain_rate[i][x][y]), | ||
ExcMessage ("Error: Averaged dviscosities_dstrain_rate is not finite.")); | ||
|
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.
Why do you separate this if-block from the one above? If you could unify them it would be easier to read the code, because all derivative stuff would be in one place.
reference_specific_heat = prm.get_double ("Reference specific heat"); | ||
thermal_alpha = prm.get_double ("Thermal expansion coefficient"); | ||
thermal_expansivity = prm.get_double ("Thermal expansion coefficient"); |
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.
align equal signs
tests/drucker_prager_derivatives.cc
Outdated
in_base.pressure[4] = 2e12; | ||
|
||
/** | ||
* We cant take to slow values, because then the difference in the |
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.
what are slow values? Do you mean small strain rates?
tests/drucker_prager_derivatives.cc
Outdated
// run this function by initializing a global variable by it | ||
int ii = f(-1000); // Testing min function | ||
int iz = f(-2); // Testing min function | ||
int ij = f(-1.5); // Testing generalized p norm mean with negative p |
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 really dont think these comments apply to these function calls 😄. Please fix all of them.
* computes the volume fractions of the compositional fields, and returns it so that the sum | ||
* of the volume fractions is one. | ||
*/ | ||
std::vector<double> compute_volume_fractions( const std::vector<double> &compositional_fields) const; |
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.
Is this function used in the .cc file? I do not see it.
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.
you are right, this comes from code when there where different compositions present. I will remove this.
@@ -76,43 +76,17 @@ namespace aspect | |||
* @ingroup MaterialModels | |||
*/ | |||
template <int dim> | |||
class DruckerPrager : public MaterialModel::InterfaceCompatibility<dim>, public ::aspect::SimulatorAccess<dim> | |||
class DruckerPrager : public MaterialModel::Interface<dim>, public ::aspect::SimulatorAccess<dim> |
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.
Please read through the class documentation above and make sure it is still accurate after this change. If not please update, and do not forget to update the string at the bottom of the .cc file.
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 don't see anything what should change based on this. Did you have anything in mind? I could add that the derivatives are added to the material model.
const double pressure, | ||
const std::vector<double> &compositional_fields, | ||
const Point<dim> &position) const; | ||
virtual void evaluate(const MaterialModel::MaterialModelInputs<dim> &in, |
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.
Thanks for the cleanup 👍
} | ||
else | ||
{ | ||
out.viscosities[i] = effective_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.
I suppose since the tests pass the viscosity calculation is unchanged? I did not go through all of the formulas, but did you intentionally change something, or should everything be as before? Except for the addition of derivatives of course.
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.
The viscosity should be exactly the same as before.
f00f38e
to
7049917
Compare
7049917
to
4159efa
Compare
split up of #1653, part 2. This pull request changes the drucker prager material model to the new material model style and adds derivatives needed for the Newton method.