Change polytropic to tab EOS for CCSN initialize #6686
Conversation
73a26dd to
1a46646
Compare
| for (size_t i = 0; i < num_radial_points_; i++) { | ||
| if (radius_[i] > target_radius) { | ||
| radius_index = i - 1; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
You can do this with a binary search, I think. https://en.cppreference.com/w/cpp/algorithm/lower_bound.html and then use https://en.cppreference.com/w/cpp/iterator/distance.html
const auto radius_index = static_cast<size_t>(std::distance(radius_.begin(),
std::lower_bound(radius.begin(), radius.end(), target_radius));| const double& central_angular_velocity, | ||
| const double& diff_rot_parameter, | ||
| const double& max_dens_ratio_interp, |
There was a problem hiding this comment.
For basic types (double, float, int, etc.) taking by value is better because a pointer (which is what a reference is) is 8 bytes while a double is 8 bytes, but with the pointer you have to resolve the pointer. Not a huge deal here, but in perf critical regions this can make a big difference.
There was a problem hiding this comment.
Clang tidy was giving me trouble on these lines, saying they should be references; do you suggest taking by value and overriding it with //NOLINT?
edit: disregard above
| const auto rest_mass_density = get<hydro::Tags::RestMassDensity<DataType>>( | ||
| variables(vars, x, tmpl::list<hydro::Tags::RestMassDensity<DataType>>{})); | ||
| const auto temperature = get<hydro::Tags::Temperature<DataType>>( | ||
| variables(vars, x, tmpl::list<hydro::Tags::Temperature<DataType>>{})); | ||
| const auto electron_fraction = | ||
| get<hydro::Tags::ElectronFraction<DataType>>(variables( | ||
| vars, x, tmpl::list<hydro::Tags::ElectronFraction<DataType>>{})); |
There was a problem hiding this comment.
Use const auto& here. const auto does a copy instead of a reference.
| const auto rest_mass_density = get<hydro::Tags::RestMassDensity<DataType>>( | ||
| variables(vars, x, tmpl::list<hydro::Tags::RestMassDensity<DataType>>{})); | ||
| const auto electron_fraction = | ||
| get<hydro::Tags::ElectronFraction<DataType>>(variables( | ||
| vars, x, tmpl::list<hydro::Tags::ElectronFraction<DataType>>{})); | ||
| const auto temperature = get<hydro::Tags::Temperature<DataType>>( | ||
| variables(vars, x, tmpl::list<hydro::Tags::Temperature<DataType>>{})); |
There was a problem hiding this comment.
Use const auto& here. const auto does a copy instead of a reference.
| const auto rest_mass_density = get<hydro::Tags::RestMassDensity<DataType>>( | ||
| variables(vars, x, tmpl::list<hydro::Tags::RestMassDensity<DataType>>{})); | ||
| const auto specific_internal_energy = | ||
| get<hydro::Tags::SpecificInternalEnergy<DataType>>(variables( | ||
| vars, x, | ||
| tmpl::list<hydro::Tags::SpecificInternalEnergy<DataType>>{})); | ||
| const auto pressure = get<hydro::Tags::Pressure<DataType>>( | ||
| variables(vars, x, tmpl::list<hydro::Tags::Pressure<DataType>>{})); |
There was a problem hiding this comment.
Use const auto& here. const auto does a copy instead of a reference.
| #include "IO/Connectivity.hpp" | ||
| #include "IO/H5/AccessType.hpp" | ||
| #include "IO/H5/CheckH5.hpp" | ||
| #include "IO/H5/EosTable.hpp" | ||
| #include "IO/H5/File.hpp" | ||
| #include "IO/H5/Header.hpp" | ||
| #include "IO/H5/Helpers.hpp" | ||
| #include "IO/H5/OpenGroup.hpp" | ||
| #include "IO/H5/SourceArchive.hpp" | ||
| #include "IO/H5/Version.hpp" | ||
| #include "IO/H5/Wrappers.hpp" |
There was a problem hiding this comment.
I don't think you need any of these includes in the hpp because the cpp file reads in the data, but the class doesn't store the h5 file, it stores the contents. Phrased differently, the hpp doesn't use any of the h5 stuff, so it's only needed in the cpp. Putting it in the cpp will help keep compile times down. In the cpp you only need
#include "IO/H5/AccessType.hpp"
#include "IO/H5/EosTable.hpp"
#include "IO/H5/File.hpp"
Proposed changes
Proper read in of tabulated EOS data to initialize CCSN executable.
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments