-
Notifications
You must be signed in to change notification settings - Fork 75
Description
There are a few follow-on issues from #1080 (not problems with that PR itself, but things that became apparent while working on it) that I am listing below so we don't forget them:
- I don't think the following is what we want to do going forward (BaseBinaryStar.cpp):
if ((m_Star1->IsOneOf({ STELLAR_TYPE::MASSLESS_REMNANT }) || m_Star2->IsOneOf({ STELLAR_TYPE::MASSLESS_REMNANT })) || p_Dt < NUCLEAR_MINIMUM_TIMESTEP) {
p_Dt = NUCLEAR_MINIMUM_TIMESTEP; // but not less than minimum
}
We may want to continue evolving merger products (which would be characterised by one component becoming the merger product and the other being a massless remnant). We don't want to take tiny timesteps in that case; on the contrary, the timestep should be driven exclusively by the dt passed back by the star which is not a massless remnant.
-
In retrospect, I think it was a suboptimal implementation decision to equate a negative semi-major axis with the binary being unbound, and we should fix that in the future. Otherwise, we end up having to use hacks such as setting the semi-major axis to small positive numbers as the binary approaches GW-driven merger.
-
I am worried that we may be resetting m_SemiMajorAxisPrev and m_EccentricityPrev multiple times per time step (e.g., what if we have mass transfer and GW emission happening simultaneously?), so these values could end up being overwritten in a way that may not match the developer's or user's expectations. The longer-term solution is to apply all changes to the orbit (from MT, tides, GWs) once per time step.
-
Now that we have a new ChooseTimestep() function in BaseBinaryStar to account for GWs, we should use the same functionality for tides.