Skip to content
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

Supernova from main sequence merger product not recorded in BSE_Supernovae #1317

Closed
SimonStevenson opened this issue Jan 6, 2025 · 4 comments · Fixed by #1319 or #1320
Closed

Supernova from main sequence merger product not recorded in BSE_Supernovae #1317

SimonStevenson opened this issue Jan 6, 2025 · 4 comments · Fixed by #1319 or #1320
Assignees
Labels
bug Something isn't working severity_moderate This is a moderately severe bug urgency_low This issue is not urgent

Comments

@SimonStevenson
Copy link
Collaborator

SimonStevenson commented Jan 6, 2025

Describe the bug
When using --evolve-main-sequence-merger TRUE to evolve main sequence merger products, I am finding that even if the merger product subsequently undergoes a supernova and forms a NS/BH, that event is not recorded in BSE_Supernovae

Label the issue
Please label the 'severity' and 'urgency' of this issue. You can choose:
urgency_low - This is a moderately urgent issue
severity_moderate - This is a minor bug with minimal impact

To Reproduce
For example, a single binary that merges can be run with

./COMPAS -n 1 --initial-mass-1 7.0 --initial-mass-2 1.5 --orbital-period 1.0 --evolve-main-sequence-merger TRUE --detailed-output TRUE

example_MS_merger_masses
example_MS_merger_types

Expected behavior
An entry in BSE_Supernovae should be generated for the merger product/all supernovae that occur.

Versioning (please complete the following information):

  • OS: MacOS v12.4
  • COMPAS v03.10.04

[EDITED TO REMOVE OTHER SYSTEM]

@SimonStevenson SimonStevenson added bug Something isn't working severity_moderate This is a moderately severe bug urgency_low This issue is not urgent labels Jan 6, 2025
@jeffriley
Copy link
Collaborator

jeffriley commented Jan 6, 2025

I believe the problem is that we flag a stellar merger in BaseBinaryStar::ResolveCommonEnvelopeEvent(), and because --evolve-main-sequence-merger TRUE was specified we continue to evolve the binary, but in BaseBinaryStar::EvaluateBinary() where we call BaseBinaryStar::EvaluateSupernovae() (two places) we first check if there was a stellar merger flagged, and if there was we bypass the call(s) to BaseBinaryStar::EvaluateSupernovae(). That would be the correct behaviour if --evolve-main-sequence-merger was not set TRUE.

@ilyamandel can you take a look? It's probably a fairly simple fix, but since you implemented --evolve-main-sequence-merger you are more familiar with the expected code path.

EDIT: It might be as simple as not flagging the stellar merger if we're evolving merger products.
EDIT Or not... mass transfer tracker history is also set if there was a merger...

@SimonStevenson if I run this:

./COMPAS -n 1 --initial-mass-1 15.0 --initial-mass-2 1.5 --orbital-period 100.0 --evolve-main-sequence-merger FALSE --detailed-output FALSE --logfile-type 'CSV'

using COMPAS v03.10.04 I get:

Stars merged: (Main_Sequence_>_0.7 -> Core_Helium_Burning) + (Main_Sequence_>_0.7 -> Main_Sequence_>_0.7)

I don't see a SN event, so no reason to put an entry in the SN file. Is that not the result you get?

@SimonStevenson
Copy link
Collaborator Author

Hi @jeffriley you're right about the other system, I just double checked it, I think I must have had a mistake in the way I was checking systems. The main issue is with the merger product.

@jeffriley
Copy link
Collaborator

@ilyamandel stand down for a bit - I'll be pushing a fix for this in the next hour or so, but it will have some questions for you...

@jeffriley jeffriley linked a pull request Jan 7, 2025 that will close this issue
@jeffriley
Copy link
Collaborator

jeffriley commented Jan 7, 2025

Fixed in #1320

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 a pull request may close this issue.

3 participants