Add Viscosity Limit to Reactive Fluid Transport#6883
Add Viscosity Limit to Reactive Fluid Transport#6883danieldouglas92 wants to merge 2 commits intogeodynamics:mainfrom
Conversation
34d16db to
feb417c
Compare
gassmoeller
left a comment
There was a problem hiding this comment.
Very nice, just two minor comments.
| { | ||
| const double porosity = std::max(in.composition[q][porosity_idx],0.0); | ||
| out.viscosities[q] *= (1.0 - porosity) * std::exp(- alpha_phi * porosity); | ||
| const double weakend_viscosity = out.viscosities[q] * (1.0 - porosity) * std::exp(- alpha_phi * porosity); |
There was a problem hiding this comment.
| const double weakend_viscosity = out.viscosities[q] * (1.0 - porosity) * std::exp(- alpha_phi * porosity); | |
| const double weakened_viscosity = out.viscosities[q] * (1.0 - porosity) * std::exp(- alpha_phi * porosity); |
| double shear_to_bulk_viscosity_ratio; | ||
| double min_compaction_visc; | ||
| double max_compaction_visc; | ||
| double min_weakened_visc; |
There was a problem hiding this comment.
I know the other variable were around already, but can you change this to make it easier to read:
| double min_weakened_visc; | |
| double min_weakened_viscosity; |
feb417c to
e3ba727
Compare
|
@gassmoeller Thanks for the quick review and for catching that spelling mistake! I opened up a quick follow up PR to change the parameter name of the compaction viscosity limiters to be consistent with the new parameter in this pr |
naliboff
left a comment
There was a problem hiding this comment.
@danieldouglas92 - Nice, thank you for putting this PR together! I only have to minor suggested changes on top of those already noted by @gassmoeller.
| @@ -0,0 +1,32 @@ | |||
| # This model tests the minimum viscosity within the reactive fluid transport model | |||
There was a problem hiding this comment.
| # This model tests the minimum viscosity within the reactive fluid transport model | |
| # This model tests that the minimum viscosity within the reactive fluid transport model |
| # weakening factor of 30. Without a minimum value for the viscosity, this would | ||
| # result in the material viscosity of 4.481e17 Pa s, which is below the minimum | ||
| # value that is specified in the Visco Plastic base material model. By setting | ||
| # the parameter Minimum weakened viscosity to 1e10 Pa s, we ensure that the viscosity |
There was a problem hiding this comment.
| # the parameter Minimum weakened viscosity to 1e10 Pa s, we ensure that the viscosity | |
| # the parameter Minimum weakened viscosity to 1e19 Pa s, we ensure that the viscosity |
e3ba727 to
7d33206
Compare
|
@naliboff Thanks for catching the typos! Just addressed your comments |
7d33206 to
0f45daf
Compare
|
I'm not sure why the indent tester is failing? I've run make indent locally and no files are changed, and the indent artifact that the tester outputs is empty |
|
/rebuild |
|
The code looks all good now. The tester is failing, because since #6881 we also check in this tester if the parameter documentation needs to be updated, and you added a new parameter here. You can update the parameter documentation manually by running |
|
The first commit worked for models that I was running (which did not use the Katz2003 melting model), but @xlia62 pointed out that it wasnt working when using the katz2003 melting model. This is because the katz2003 melting model calculates this weakening factor on it's own within reaction_model/katz2003_melting_model.cc. I pushed a second commit which removes this code from katz2003_melting_model.cc and moves the code in reactive_fluid_transport.cc around so that the weakening is applied regardless of the reaction model being used. I think this makes sense to avoid needing to potentially make individual viscosity limiters for each reaction model, and I think this should just work, but we'll see if the testers pass |
|
@gassmoeller Ah that makes sense! I'll push more changes in a minute with the updated parameter. |
20bbb07 to
614225d
Compare
614225d to
8ea5508
Compare
|
The changes to katz2003 caused two of the tests ( Clearly this is why there was an if statement in reactive_fluid_transport that excluded the fluid weakening from being calculated if the reaction model was equal to katz2003. Because the katz2003 interfaced with material models outside of reactive fluid transport, the weakening had to be determined internally within the reaction model itself. An easy solution to this would be to just add an identical parameter to the katz2003 reaction model that allows the user to limit the reduction of the shear viscosity, but this feels pretty hacky. |
In the
evaluatefunction ofReactive fluid transportmodel, the base material model is first evaluated and then a fluid weakening term is multiplied to the viscosity computed in the base model. For base models likeVisco plastic, this means that the viscosity that is returned byReactive fluid transportcan be much lower than theMinimum viscosityparameter set within the base model.This PR adds a new parameter to the
Reactive fluid transportmodel calledMinimum weakened viscositythat sets a lower bound on the viscosity after applying this fluid weakening factor. I was hoping that there might be a better way to do this without defining an essentially duplicated parameter, but the problem is that not all base models contain a parameter that sets a minimum viscosity value (i.e. theSimplematerial model). If there is an alternative way to generalize this, I would love to hear other ideas!