search

TeamCOMPAS / COMPAS

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

split up macleod linear AM loss fraction for degenerate and non-degen… #1136

Closed reinhold-willcox closed 4 months ago

reinhold-willcox commented 4 months ago

…erate accretors

ilyamandel commented 4 months ago

@reinhold-willcox , @jeffriley -- I didn't have time to check this yesterday, but there's a bug in this PR that needs to be fixed.

The default values in the code and in the YAML file do not match the values in the documentation:

m_MassTransferJlossMacLeodLinearFractionDegen                   = 0.0;
m_MassTransferJlossMacLeodLinearFractionNonDegen                = 0.0;

vs.

**--mass-transfer-jloss-macleod-linear-fraction-degen** |br|
Specific angular momentum interpolation fraction for degenerate accretors, linear between 0 and 1 corresponding to the accretor and L2 point. |br|
Used when ``--mass-transfer-angular-momentum-loss-prescription = MACLEOD_LINEAR``, ignored otherwise. |br|
Default = 0.5

Could you please fix this discrepancy? I believe in this case the documentation describes the expected defaults, so the code needs to be updated and the YAML file needs to be regenerated.

Also, since you've removed one of the previously existing options, --mass-transfer-jloss-macleod-linear-fraction, this may merit an entry in the whatsnew.

jeffriley commented 4 months ago

I'll fix this - I'm about to push some other changes. I should have noticed this in my review - I rushed it...

Jeff

-- Dr Jeff Riley Monash University, Australia Ph: +61 (0)407 811 282 @.***

On Thu, 23 May 2024 at 10:52, Ilya Mandel @.***> wrote:

@reinhold-willcox https://github.com/reinhold-willcox , @jeffriley https://github.com/jeffriley -- I didn't have time to check this yesterday, but there's a bug in this PR that needs to be fixed.

The default values in the code and in the YAML file do not match the values in the documentation:

m_MassTransferJlossMacLeodLinearFractionDegen = 0.0; m_MassTransferJlossMacLeodLinearFractionNonDegen = 0.0;

vs.

--mass-transfer-jloss-macleod-linear-fraction-degen |br|

Specific angular momentum interpolation fraction for degenerate accretors, linear between 0 and 1 corresponding to the accretor and L2 point. |br| Used when --mass-transfer-angular-momentum-loss-prescription = MACLEOD_LINEAR, ignored otherwise. |br| Default = 0.5

Could you please fix this discrepancy? I believe in this case the documentation describes the expected defaults, so the code needs to be updated and the YAML file needs to be regenerated.

Also, since you've removed one of the previously existing options, --mass-transfer-jloss-macleod-linear-fraction, this may merit an entry in the whatsnew.

— Reply to this email directly, view it on GitHub https://github.com/TeamCOMPAS/COMPAS/pull/1136#issuecomment-2126002613, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGM5XNS4AUOL62U22YOGMSLZDU4WLAVCNFSM6AAAAABIDBDATSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRWGAYDENRRGM . You are receiving this because you were mentioned.Message ID: @.***>