Refactor the Tian Approximation in the Reactive Fluid Transport Model #6161
Conversation
danieldouglas92
force-pushed
the
refactor_reactive_fluid_transport_model
branch
from
af3a3e0 to
f10d604
Compare
include/aspect/material_model/reaction_model/tian2019_solubility.h
Outdated
Show resolved
Hide resolved
include/aspect/material_model/reaction_model/tian2019_solubility.h
Outdated
Show resolved
Hide resolved
include/aspect/material_model/reaction_model/tian2019_solubility.h
Outdated
Show resolved
Hide resolved
include/aspect/material_model/reaction_model/tian2019_solubility.h
Outdated
Show resolved
Hide resolved
include/aspect/material_model/reaction_model/tian2019_solubility.h
Outdated
Show resolved
Hide resolved
include/aspect/material_model/reaction_model/tian2019_solubility.h
Outdated
Show resolved
Hide resolved
| std::vector<double> LR_peridotite_poly_coeffs {-19.0609, 168.983, -630.032, 1281.84, -1543.14, 1111.88, -459.142, 95.4143, 1.97246}; | ||
| std::vector<double> csat_peridotite_poly_coeffs {0.00115628, 2.42179}; | ||
| std::vector<double> Td_peridotite_poly_coeffs {-15.4627, 94.9716, 636.603}; | ||
|
|
||
| std::vector<double> LR_gabbro_poly_coeffs {-1.81745, 7.67198, -10.8507, 5.09329, 8.14519}; | ||
| std::vector<double> csat_gabbro_poly_coeffs {-0.0176673, 0.0893044, 1.52732}; | ||
| std::vector<double> Td_gabbro_poly_coeffs {-1.72277, 20.5898, 637.517}; | ||
|
|
||
| std::vector<double> LR_MORB_poly_coeffs {-1.78177, 7.50871, -10.4840, 5.19725, 7.96365}; | ||
| std::vector<double> csat_MORB_poly_coeffs {0.0102725, -0.115390, 0.324452, 1.41588}; | ||
| std::vector<double> Td_MORB_poly_coeffs {-3.81280, 22.7809, 638.049}; | ||
|
|
||
| std::vector<double> LR_sediment_poly_coeffs {-2.03283, 10.8186, -21.2119, 18.3351, -6.48711, 8.32459}; | ||
| std::vector<double> csat_sediment_poly_coeffs {-0.150662, 0.301807, 1.01867}; | ||
| std::vector<double> Td_sediment_poly_coeffs {2.83277, -24.7593, 85.9090, 524.898}; |
There was a problem hiding this comment.
All of these numbers are fixed and will never change during the program runtime. You can tell this to the compiler, which will then optimize the code further and also prevent you from accidentally changing the values by changing their type from std::vector<double> to constexpr std::array<double,N> where N is the number of values in this array. Initialization and access to these arrays can remain unchanged.
There was a problem hiding this comment.
I had tried to do this with arrays originally but I ultimately ran into issues because all of the arrays are different lengths. Because I ultimately end up storing each of these poly_coeffs arrays into a vector farther down (Lines 138-148). It seems like there might be a way forward with tuples, when I tried this the looping over each poly_coeffs array seemed a little strange and this type of approach doesn't seem like it's done anywhere else in the code.
I think the only way to get around the way that I'm currently storing these constant values would be to alter the way the code within the tian_equilibrium_bound_water_content function. But because all of these arrays are not necessarily the same size, I'm not sure that there's a good way to do this without adding a bunch of if statements.
| * and 50 GPa for sediment. These cutoff pressures were determined by extending the pressure range in Tian et al. (2019) | ||
| * and observing where the maximum allowed water contents jump towards infinite values. | ||
| */ | ||
| const std::vector<double> pressure_cutoffs {10, 26, 16, 50}; |
There was a problem hiding this comment.
same here, make this a constexpr std::array
There was a problem hiding this comment.
This one vector can be made into a constexpr because it doesn't get stored in another vector thankfully!
| prm.enter_subsection("Tian 2019 model"); | ||
| { | ||
| tian2019_model.initialize_simulator (this->get_simulator()); | ||
| tian2019_model.parse_parameters(prm); | ||
| } | ||
| prm.leave_subsection(); |
There was a problem hiding this comment.
So you put the parameters of the Tian model in their own subsection now. However, this means the parameter files of everyone using the reactive fluid flow model with the 3.0 version will need to be updated. Please include a changelog entry that mentions this. Additionally, you could extend our update scripts in contrib/utilities/update_scripts/prm_files/ to automatically move this parameter. The closest operation that does something like this is probably this function. However, the update process looks a bit cryptic, I leave this up to you to decide if this is important enough.
There was a problem hiding this comment.
I'll take a stab at this!
danieldouglas92
force-pushed
the
refactor_reactive_fluid_transport_model
branch
from
f10d604 to
0b13719
Compare
danieldouglas92
force-pushed
the
refactor_reactive_fluid_transport_model
branch
from
7d22a94 to
b7da80c
Compare
|
@gassmoeller Thanks for the review, besides the point about creating std::arrays I've addressed all your points, created a python script to modify the input file and automatically create a new subsection for the Tian 2019 reaction model. This is now ready for another review when you get the chance! |
danieldouglas92
force-pushed
the
refactor_reactive_fluid_transport_model
branch
from
6b961b4 to
215f207
Compare
|
/rebuild |
I was looking through the Reactive fluid transport model code and saw how the katz 2003 model is in it's own reaction model. I think I recall at the hackathon that the ultimate goal for this material model was to expand it by creating new reaction models, so I went and refactored the tian approximation into it's own reaction.