-
Notifications
You must be signed in to change notification settings - Fork 2
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
calculating Mass accretion rate for SN post-processing #211
Conversation
src/progenitor/progenitordata.cpp
Outdated
ReduceInGain<class internal_variables::GcovCool>(md, 0, 0); | ||
}; | ||
auto Mdot400 = [](MeshData<Real> *md) { | ||
Real rc = 0.04; |
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 make this a runtime set-able parameter. Might even be good to support an arbitrary number of these at different radii.
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.
I am trying to imagine how is the second part going to work
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.
ParameterInput
can read a std:::vector
so you could do something like this:
auto mdot_radii = pin->GetOrAddVector("analysis", "mdot_radii", std::vector<Real>{0.04});
for (auto rc : mdot_radii) {
auto mdot = [=](MeshData<Real> *md) { return History::CalculateMdot(md, rc, 0); };
hst_vars.emplace_back(HistoryOutputVar(HstSum, mdot, "Mdot at r = " + std::to_string(rc)));
}
src/progenitor/progenitordata.cpp
Outdated
return ReduceInGain<class internal_variables::GcovHeat>(md, 0) - | ||
ReduceInGain<class internal_variables::GcovCool>(md, 0); | ||
return ReduceInGain<class internal_variables::GcovHeat>(md, 0, 0) - | ||
ReduceInGain<class internal_variables::GcovCool>(md, 0, 0); |
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 word class
here actually required?
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.
deleted and still compiles and runs so is not required.
src/analysis/history.hpp
Outdated
bool is_netheat = (v(b, iv::GcovHeat(), k, j, i) - v(b, iv::GcovCool(), k, j, i) > | ||
1.e-8); // checks that in the gain region | ||
auto analysis = pmb->packages.Get("analysis").get(); | ||
const Real outside_pns_threshold = analysis->Param<Real>("outside_pns_threshold"); |
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.
This needs to be moved outside of the kernel. It can't be called inside the par for.
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.
done
src/analysis/history.cpp
Outdated
const F &f) { | ||
Real div_mass_flux_integral; | ||
div_mass_flux_integral = | ||
-(f(b, 1, k, j, i + 1) - f(b, 1, k, j, i)) * coords.FaceArea<X1DIR>(k, j, 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.
Remove the minus signs here, and add them to the value in mdot
as the divergence doesn't have a minus sign in its definition but mdot
does, due to the orientation of surface normals.
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.
done
src/analysis/history.cpp
Outdated
auto rad = pmb->packages.Get("radiation").get(); | ||
const parthenon::AllReduce<bool> *pdo_gain_reducer = | ||
rad->MutableParam<parthenon::AllReduce<bool>>("do_gain_reducer"); | ||
const bool do_gain = pdo_gain_reducer->val; |
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.
This can't be here---you can't access packages from within a par for on GPU. Move this above.
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.
done
src/analysis/history.cpp
Outdated
auto analysis = pmb->packages.Get("analysis").get(); | ||
const Real inside_pns_threshold = | ||
analysis->Param<Real>("inside_pns_threshold"); |
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't be here. Move this out of the kernel.
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.
done
src/analysis/analysis.cpp
Outdated
Real outside_pns_threshold = | ||
pin->GetOrAddReal("analysis", "outside_pns_threshold", | ||
2.42e-5); // corresponds to entropy > 3 kb/baryon | ||
Real inside_pns_threshold = pin->GetOrAddReal("analysis", "inside_pns_threshold", | ||
0.008); // corresponds to r < 80 km | ||
Real radius_cutoff_mdot = | ||
pin->GetOrAddReal("analysis", "radius_cutoff_mdot", 0.04); // default 400km |
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.
This stuff might better belong in the progenitor package.
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.
moved to progenitor pkg
src/progenitor/progenitordata.cpp
Outdated
ReduceInGain<class internal_variables::GcovCool>(md, 0, 0); | ||
}; | ||
auto Mdot400 = [](MeshData<Real> *md) { | ||
Real rc = 0.04; |
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.
ParameterInput
can read a std:::vector
so you could do something like this:
auto mdot_radii = pin->GetOrAddVector("analysis", "mdot_radii", std::vector<Real>{0.04});
for (auto rc : mdot_radii) {
auto mdot = [=](MeshData<Real> *md) { return History::CalculateMdot(md, rc, 0); };
hst_vars.emplace_back(HistoryOutputVar(HstSum, mdot, "Mdot at r = " + std::to_string(rc)));
}
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.
This is looking good! A few more minor changes...
src/progenitor/progenitordata.cpp
Outdated
Real LengthCGSToCode = units.GetLengthCGSToCode(); | ||
Real rc = 400e5 * LengthCGSToCode; | ||
auto mdot_radii = pin->GetOrAddVector("progenitor", "mdot_radii", | ||
std::vector<Real>{rc}); // default 400km |
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.
I don't think you want to conversion here.
Real LengthCGSToCode = units.GetLengthCGSToCode(); | |
Real rc = 400e5 * LengthCGSToCode; | |
auto mdot_radii = pin->GetOrAddVector("progenitor", "mdot_radii", | |
std::vector<Real>{rc}); // default 400km | |
Real LengthCGSToCode = units.GetLengthCGSToCode(); | |
auto mdot_radii = pin->GetOrAddVector("progenitor", "mdot_radii", | |
std::vector<Real>{400}); // default 400km |
src/progenitor/progenitordata.cpp
Outdated
auto Mdot = [rc, LengthCGSToCode](MeshData<Real> *md) { | ||
return History::CalculateMdot(md, rc, 0); |
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.
auto Mdot = [rc, LengthCGSToCode](MeshData<Real> *md) { | |
return History::CalculateMdot(md, rc, 0); | |
auto rc_code = rc*1e5*LengthCGSToCode; | |
auto Mdot = [rc_code](MeshData<Real> *md) { | |
return History::CalculateMdot(md, rc_code, 0); |
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.
As soon as tests pass here (and you confirm it works for you locally) I will merge.
All tests have passed, compiles, runs, produces reasonable output. |
PR Summary
PR Checklist
scripts/bash/format.sh
.