-
Notifications
You must be signed in to change notification settings - Fork 70
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
Incorrect brief evolutionary outputs printed to screen #1286
Comments
@ilyamandel the output is correct - it's only reporting what happened during the evolution of the binary. The problem, I think, is that we are not flagging the merger early enough in We set the stellar merger flag here:
but that is after we've allowed both stars to call
but we don't... ( |
Ok... FGB stars have CONVECTIVE envelopes... so we don't consider ot a stellar merger at that point - hence both stars continue to evolve (and switch stellar type), then we set the stellar merger flag. So the stars started as MS stars, and ended evolution as HeWD stars, with the merger happening (according to where we set the flag) after they both switched to HeWD. Is the earlier check for stellar merger correct? |
Physically, the stars merge while they are FGB stars. |
I get that, but that's not what's happening in the code... So, the output is correct, but COMPAS evolution is not. If we want to report what should be happening, we need to set the stellar merger flag earlier and not let the stars evolve to HeWD stars. EDIT: ...the output is correct (wrt to the code) |
The output is always correct with respect to the code: the computer can only do what we tell it to do. ;-) What I meant was that users may care whether stars merge as FGB stars or as WDs (e.g., the mergers would look very different observationally), so we should fix how we report these mergers. |
I agree - but my point is that the code as it stands allows the stars to continue evolving after (what we are saying should be) the stellar merger (so it's not that we are reporting the result incorrectly, we are calculating the result incorrectly) - because we don't recognise that the merger has happened at the right place in the code. I think the condition noted above for setting the merger flag is either wrong or deficient - but I don't know what it should be :-) EDIT: you'll see in the code that if the merger flag is set correctly at the condition noted, the code that evolves the stars forward to HeWD stars is not executed, so the status printed will be correct (the stars will be shown as FGB stars) |
@jeffriley -- not as easy as I expected: I seemingly fixed this in #1291, and for the example in the original issue report I now see that m_Flags.stellarMerger is true and both types are still FGB at the end of BaseBinaryStar::ResolveCommonEnvelopeEvent(). |
After #1291 , these are being recorded as massless remnants in the brief screen output, but are correctly labeled as a CE between two FGB stars. Maybe there's a better way to present those brief screen outputs, but happy with the existing solution for now, so closing. |
The brief outcome of the evolution printed to the screen is sometimes incorrect. For example, the following
./COMPAS -n 1 --initial-mass-1 1 --initial-mass-2 1 --semi-major-axis 0.03 --detailed-output --logfile-type CSV --evolve-main-sequence-mergers
leads to the output
0: Stars merged: (Main_Sequence_>0.7 -> Helium_White_Dwarf) + (Main_Sequence>_0.7 -> Helium_White_Dwarf)
In fact, the stars merge earlier, as giants, not as HeWDs.
The text was updated successfully, but these errors were encountered: