TeamCOMPAS / COMPAS

COMPAS rapid binary population synthesis code
http://compas.science
MIT License
64 stars 66 forks source link

Birth stars touching at periapsis not treated as merger #309

Closed themikelau closed 4 years ago

themikelau commented 4 years ago

In the BaseBinaryStar constructor in BaseBinaryStar.cpp, the "merger" condition (bool) is computed as

merger                                 = (m_SemiMajorAxis * AU_TO_RSOL) < (m_Star1->Radius() + m_Star2->Radius());

For an binary that is eccentric at birth, this would allow for periapsis contact. Instead, we should have something like

merger                                 = (m_SemiMajorAxis * AU_TO_RSOL * ( 1.0 - m_Eccentricity * m_Eccentricity)) < (m_Star1->Radius() + m_Star2->Radius());

This bug would not have and will not affect any runs that assume only circular binaries are born, but would possibly affect runs made assuming some other eccentricity distribution (e.g. thermal eccentricity distribution).

ilyamandel commented 4 years ago

Nice catch, @themikelau, but two points :

  1. Touching is a temporary proxy for L2 overflow (see my comments in issue #344 ). We should eventually substitute the calculation of L2 overflow for testing for "touching", including at the start.

  2. We allow for initially eccentric binaries in contact to circularise through mass transfer if the appropriate flags are set, which may (depending on settings, whether they are CHE candidates, etc.), which may involve the conservation of angular momentum, i.e., the periapsis may grow -- so stars which were previously "touching" are no longer so. Now, whether this should be possible for stars which start out in a configuration leading to L2 overflow is an interesting question, but I suggest we leave this for a more consistent solution, i.e., roll this issue into #344 .

Therefore, closing for now, and updating #344 to explicitly mention initial configuration.