Skip to content

Conversation

@jeffriley
Copy link
Collaborator

Fix for issue 1317 - SN event not always logged to BSE log file when evolving MS merger products.

@ilyamandel I thought I had some questions about where we need to check for evolving MS merger products when there has been a stellar merger, but upon checking I think we're covered in all places, so the fix here should be all that's needed.

I am probably responsible for this problem manifesting - I have a vague recollection of adding a check for stellar merger towards the end of BaseBinaryStar::EvaluateBinary() thinking, at the time, "We don't need to do this if we've had a stellar merger". Well, we do if the stellar merger was a MS merger and we're evolving MS merger products... This is the check, now modified, at (now) line 2911 in BaseBinaryStar.cpp.

While investigating this problem I noticed that we don't always log a "TIMESTEP_COMPLETED" record to the BSE_Detailed_Output file for the very last timestep. We do log a "FINAL_STATE" record which has all the info required, but users (and particularly I think the detailed plotting code) look for the "TIMESTEP_COMPLETED" record type - so I changed the code to ensure a "TIMESTEP_COMPLETED" record is always logged, even if has the same information as the "FINAL_STATE" record.

@jeffriley jeffriley requested a review from ilyamandel January 7, 2025 23:24
@jeffriley jeffriley added bug Something isn't working severity_moderate This is a moderately severe bug urgency_low This issue is not urgent labels Jan 7, 2025
@jeffriley jeffriley self-assigned this Jan 7, 2025
@SimonStevenson
Copy link
Collaborator

Thanks for the fix Jeff, I'll check it out. Is the target branch right on this PR? Don't you want to merge into dev?

@jeffriley jeffriley merged commit 4264049 into TeamCOMPAS:issue-1317 Jan 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working severity_moderate This is a moderately severe bug urgency_low This issue is not urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Supernova from main sequence merger product not recorded in BSE_Supernovae

2 participants