-
Notifications
You must be signed in to change notification settings - Fork 235
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
Viscosity in no melting/freezing set-up #2967
Viscosity in no melting/freezing set-up #2967
Conversation
source/material_model/melt_global.cc
Outdated
@@ -157,16 +157,19 @@ namespace aspect | |||
0.0; | |||
out.densities[i] = (reference_rho_s + delta_rho) * temperature_dependence | |||
* std::exp(compressibility * (in.pressure[i] - this->get_surface_pressure())); | |||
|
|||
AssertThrow(this->introspection().compositional_name_exists("porosity"), |
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 may break some things, because the Assert is now executed even if this->include_melt_transport() == false
and in this case the model may not have a porosity field.
So what I would suggest is to split this up and put the logic back into the loop, but change the condition to
if (this->include_melt_transport())
, and then have a separate if-statement before line 175 (// Calculate the melting rate...
) saying if (include_melting_and_freezing && in.strain_rate.size())
.
Then you can compute the viscosity as out.viscosities[i] = eta_0;
outside of the loop, out.viscosities[i] *= exp(- alpha_phi * porosity);
for models with melt transport, and additionally multiply with out.viscosities[i] *= depletion_strengthening;
for models that also have melting and freezing.
Sorry for not noticing this earlier.
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.
Since it's a "global melt", it would makes sense to require a porosity field to exist anyway, no? But maybe then it would needs to go somewhere else?
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.
OK, we actually need it for one cookbook! So let's keep it. I'm trying to modify to put the different if statements.
when operator splitting
/rebuild |
The astyle tester complains that the indentation is not correct? Do you know how to fix that? |
Alright, this changes the output of one of the tests. I looked at the test, and it makes sense that this output changes (it is a test that uses the melt global model with melt, but no melting and freezing, so the viscosity is slightly lower now, which makes the velocities slightly higher). So in order for the tests to pass, you have to update the test output. You can do that in the same way as for the indentation changes (going to the tester, downloading the .diff file with the changes, and applying it using patch). |
@MarineLasbleis when you address some comments, always leave a comment about it, otherwise we will not see it. @jdannberg: This is ready for you to merge. |
Yes, sorry! |
Looks good! |
In the case of no melting/freezing, it's still possible to have a non-zero porosity (if some at the initial conditions or boundary conditions)
Since porosity is used in both cases of the if/else, then we grab the porosity value outside of the loop.