[WIP]Add EntropyReader Class #5334
Conversation
RanpengLi
force-pushed
the
entropy-reader
branch
from
0fb4bd6
to
81a5d7c
Compare
|
/rebuild |
There was a problem hiding this comment.
Some comments for now. As you told me further reviews should happen on #5341, we will continue there when you implemented the comments. Feel free to close this PR when ready.
| @@ -18,7 +18,6 @@ | |||
| <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
|
|
|||
| lateral_viscosity_prefactor_lookup = std::make_unique<internal::LateralViscosityLookup>(data_directory+lateral_viscosity_file_name, | ||
| this->get_mpi_communicator()); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
We keep 3 empty lines between functions in .cc files, and 1 empty line between them in .h files. Please add the lines here and below.
| template <int dim> | ||
| bool | ||
| EntropyModel<dim>:: | ||
| is_compressible () const |
| @@ -216,17 +211,15 @@ namespace aspect | |||
| // fill seismic velocities outputs if they exist | |||
| if (SeismicAdditionalOutputs<dim> *seismic_out = out.template get_additional_output<SeismicAdditionalOutputs<dim>>()) | |||
| { | |||
| seismic_out->vp[i] = material_lookup->get_data(entropy_pressure,4); | |||
| seismic_out->vs[i] = material_lookup->get_data(entropy_pressure,5); | |||
| seismic_out->vp[i] = entropy_reader-> seismic_vp(entropy, pressure); | |||
There was a problem hiding this comment.
| seismic_out->vp[i] = entropy_reader-> seismic_vp(entropy, pressure); | |
| seismic_out->vp[i] = entropy_reader->seismic_vp(entropy, pressure); |
| @@ -467,7 +456,6 @@ namespace aspect | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| /** | ||
| * Gets density gradient for a given entropy and pressure | ||
| */ | ||
| Tensor<1, 2> | ||
| calc_density_gradient(const double entropy, | ||
| const double pressure) const; |
There was a problem hiding this comment.
The name of this function deviates from the pattern you established before. By prefixing it with calc_ it suggests that there is a more complicated operation to perform. However the operation is not much more difficult (it simply calls get_gradient instead of get_value of the StructuredDataLookup class). Therefore I would suggest you just use the same naming scheme, which would mean density_gradient would be sufficient as name.
| std::string data_directory; | ||
| std::string material_file_name; |
There was a problem hiding this comment.
Are these parameters actually needed outside of the initialize function? If not you dont need to make them member variables and can just use them as temporary variables inside the initialize function.
Edit: In fact it looks like you do not even set these variables to any value, so you can just delete them.
| @@ -487,59 +566,58 @@ namespace aspect | |||
| * density or viscosity). | |||
| */ | |||
| template <int dim> | |||
| class PhaseFunction: public ::aspect::SimulatorAccess<dim> | |||
| class PhaseFunction : public ::aspect::SimulatorAccess<dim> | |||
| material_lookup->load_file(data_directory+material_file_name, | ||
| comm); | ||
| } | ||
|
|
There was a problem hiding this comment.
three empty lines here and between all other function implementations
| Point<2> entropy_pressure(entropy, pressure); | ||
| const double specific_heat = material_lookup->get_data(entropy_pressure, 3); |
There was a problem hiding this comment.
I know you moved this from another place, but we now have a better way to write this:
| Point<2> entropy_pressure(entropy, pressure); | |
| const double specific_heat = material_lookup->get_data(entropy_pressure, 3); | |
| const double specific_heat = material_lookup->get_data({entropy,pressure}, 3); |
This feature is called an initializer_list and used to create the Point implicitly, so we save a line of code (and the point is only available inside of the get_data function.
You can change this for all places below as well.
I added an EntropyReader class which reads in entropy-pressure table and gets material properties for a given entropy and pressure. This part was previously done in the entropy plugin.