-
Notifications
You must be signed in to change notification settings - Fork 77
Ge qcrit table He #1280
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
Ge qcrit table He #1280
Conversation
|
@reinhold-willcox I haven't looked at the changes in detail yet, but just a heads-up that we have code to deprecate options and option values - see
and explanation towards the top of If you update those vectors, the COMPAS option parsing code will replace any deprecated options/option values before passing the options to Boost for parsing, and also emit a warning that the user has specified deprecated options/values and show the new options/values to be used instead. That way we ease users into using the new options/values instead of just cutting them off. The idea is to then remove the entries from the vector after some grace (deprecation) period (maybe 6 months). (Removing the entries is a manual process - we could automate it by adding a date to the vectors, but then they'd just grow...). |
ilyamandel
left a comment
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.
HeMS.cpp -- comment does not match function (the line that says it has no input is correct)
- Function takes no input (unlike in the H-rich case) because the existing table only applies for fully conservative
- mass transfer and the GE fully adiabatic response, not the artificially isentropic one. Also only for Z=Zsol.
- Interpolation is done linearly in logM and logR.
- double BaseStar::InterpolateGeEtAlQCrit(const QCRIT_PRESCRIPTION p_qCritPrescription, const double p_massTransferEfficiencyBeta)
- @param [IN] p_qCritPrescription Adopted critical mass ratio prescription
- @param [IN] p_massTransferEfficiencyBeta Mass transfer accretion efficiency
- @return Interpolated value of either the critical mass ratio or zeta for given stellar mass / radius
*/
In constants.h and elsewhere, would be good to say exactly from which Ge+ paper (or, if not, private comm) the various values come; ditto for the functions in MainSequence.cpp and HeMS.cpp.
Thanks for catching the header mistake, I'll fix that. The papers are referenced in constants.h though, in the header above the tables (I can move them somewhere clearer if you think that would help). @jeffriley Thanks for the heads up, I'll add those in hopefully later this week. |
|
Build Successful! You can find a link to the downloadable artifact below.
|
ilyamandel
left a comment
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.
Thank you for the fixes, @reinhold-willcox ! Looks good to me, so I am approving, but please don't merge until you've addressed the issue pointed out by @jeffriley .
|
@jeffriley I think I've addressed your fixes, let me know if I missed anything |
jeffriley
left a comment
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.
All good - thanks @reinhold-willcox
Following a suggestion from the Google Users group, I've implemented here the latest MT stability prescriptions from Hongwei Ge's team, specifically their "Adiabatic Mass Loss" series. These come in the form of tables supplying the critical mass ratio as a function of donor mass and radius.
Thus we now interpolate linearly in log(Mass), log(Radius), log(Metallicity), and accretion efficiency for H-rich stars, and in log(Mass) and log(Radius) for He-stars. Extrapolating outside of the supplied range results in a nearest neighbor approximation.
Documentation and input options have been modified to reflect that we are no longer just using GE et al. 2020. Thus this is a minor breaking change - anyone using
--mass-transfer-stability-prescription GE20previously will now have to change it to simplyGE, and similarly forGE20_IC.Notably, the He star prescriptions should be used with caution. Some preliminary testing showed that the radial evolution of He stars in the Ge et al. models differed by quite a bit, in some cases, and that the mass of a He star in COMPAS can sometimes be quite a bit larger than the upper mass limit provided by these models (10 Msun). Nonetheless, this is a useful step toward having a more nuanced treatment of stability for He stars.