-
Notifications
You must be signed in to change notification settings - Fork 251
[WIP] new melt formulation #6344
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
base: main
Are you sure you want to change the base?
Conversation
source/simulator/melt.cc
Outdated
* scratch.phi_p[i] * scratch.phi_p_c[j] | ||
+ | ||
(p_c_scale * one_over_eta * | ||
(one_over_eta * |
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.
we will need to rename scratch.phi_p_c ...
source/simulator/melt.cc
Outdated
const double K_D = melt_outputs->permeabilities[q] / melt_outputs->fluid_viscosities[q]; | ||
const double viscosity_c = melt_outputs->compaction_viscosities[q]; | ||
const double e = 1.e-5 / viscosity_c; | ||
const double regularization = 1./std::sqrt(1./(viscosity_c*viscosity_c)+e*e); |
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 the 1/ correct?
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.
can you multiply with regularization
below and remove the outer 1/ ?
(K_D * | ||
pressure_scaling * | ||
pressure_scaling) * | ||
scratch.grad_phi_p[i] * |
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.
Line 230:
replace one_over_eta with one_over_viscosity_c_e
source/simulator/melt.cc
Outdated
scratch.grad_phi_p[j] | ||
+ | ||
(1./eta + 1./viscosity_c) * p_c_scale * p_c_scale * | ||
(1./eta + 1./viscosity_c) * |
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.
Maybe 1./viscosity_c needs to be -1./viscosity_c
Edit: Nevermind, it should stay a plus
source/simulator/melt.cc
Outdated
data.local_matrix(i,j) += | ||
( | ||
(p_c_scale * one_over_eta * | ||
(one_over_eta * |
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.
times (-1) and this should be one_over_viscosity_c
source/simulator/melt.cc
Outdated
* scratch.phi_p[i] * scratch.phi_p_c[j] | ||
+ | ||
(p_c_scale * one_over_eta * | ||
(one_over_eta * |
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.
times (-1) and this should be one_over_viscosity_c
source/simulator/melt.cc
Outdated
data.local_matrix(i,j) += | ||
( | ||
(p_c_scale * one_over_eta * | ||
(-1./viscosity_c * |
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 the e_squared here.
eeeb98f
to
db56e0b
Compare
This is a first try to change our melt transport formulation to the one from https://doi.org/10.1029/2023JB027469
@tjhei Can you have a look? I used the melt_transport_convergence_test to test the implementation, but the results are quite wrong (the velocities a little bit, the pressures by a lot). For now, I also hard-coded e to be 1.e-5 / viscosity_c; in the paper they use 1.e-10 / viscosity_c as far as I can tell, but if I make e any smaller the solver does not converge any more. That is even though this problem never goes to small melt fractions, so we should not even need any regularization. And for the testcase, I thought that even if I made a mistake in the computation of the pressure somehow, at least the solid velocity should still be correct, which it isn't.
EDIT: We now think the solution is correct, based on the fact that we get the correct velocities and fluid pressure for the melt_transport_convergence_simple test. The other pressures are off by a constant factor, probably due to the pressure normalization.
Now the question is: Is this new formulation actually better than what we have?
One issue is that we cannot normalize the fluid pressure any more, because it explicitly shows up in the equations. In addition, the fluid pressure is defined to be zero in regions without melt, which makes it jump at the melt/no melt boundary. Therefore, it might be better to apply the regularization to both p_f and p_t in the second equation. This would make the fluid pressure equal to the total pressure for zero porosity (no jump) and we can again normalize the pressure because only p_t - p_f appears in the equations, so we could change both by the same amount without changing the solution otherwise.
This still means that in the case of zero porosity, the second equation would be eps (p_t - p_f) = 0 with a small eps (in the paper, they choose 1e-10). We need to see if that is a problem for the linear solver.
Other things we still need to do:
Test cases to do:
Already done:
For new features/models or changes of existing features: