TeamCOMPAS / COMPAS

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

High mass black holes receiving no kick, even when using options that should allow for kicks #1016

Closed SimonStevenson closed 6 months ago

SimonStevenson commented 1 year ago

Describe the bug I noticed that when using something like the following set of options:

./COMPAS -n 1000000 --metallicity-distribution LOGUNIFORM --evolve-unbound-systems FALSE --chemically-homogeneous-evolution NONE --allow-rlof-at-birth FALSE --pulsational-pair-instability-prescription FARMER --kick-magnitude-distribution MAXWELLIAN --black-hole-kicks REDUCED

that black holes above ~30 Msun still get no kicks

So far, I think this may be a conflict between --black-hole-kicks and --pulsational-pair-instability-prescription FARMER

Label the issue Please label the 'severity' and 'urgency' of this issue. You can choose:

urgency_moderate - This is a moderately urgent issue

severity_moderate - This is a moderately severe bug

To Reproduce Run a small population. Look at black hole kicks Drawn_Kick_Magnitude(SN) and 'Applied_Kick_Magnitude(SN)' in BSE_Supernova

Expected behavior If user allows black holes to have kicks, they should have kicks

Screenshots

applied_kicks_v_mass drawn_kicks_v_mass

Versioning (please complete the following information): MacOS v 12.4 COMPAS v02.40.00

Additional context Add any other context about the problem here.

ilyamandel commented 1 year ago

@SimonStevenson -- I don't think it's a bug, more like an intentional choice: PISNe and PPISNe don't result in natal kicks (see, e.g., BaseStar::CalculateSNKickMagnitude() ). Do you have an alternative proposal?

SimonStevenson commented 1 year ago

Thanks, you're right that this does look deliberate. PISNe don't produce BHs, so clearly they shouldn't impart kicks. But BHs formed via PPISNe still undergo core-collapse (I know we classify them as distinct things, but really it's one followed by the other), and they have the same uncertainty on their kick magnitudes as any other massive BHs. That's something I was trying to play with, and the code as it currently is, doesn't allow that flexibility. I'm fine with PPISN BHs having no kicks being the default assumption, but when I explicitly set an option to give black holes kicks I'd like the code to do what I asked.

ilyamandel commented 1 year ago

Hi @SimonStevenson , PPISNe presumably experience multiple pulsations. If so, the kick should be a stochastic combination of multiple smaller kicks. So we would need to make some reasonable assumptions regarding these pulsations, which, I think, is why we didn't assign kicks to PPISNe in the past. (Note that the distribution of mass loss among multiple pulsations should impact Blaauw kicks, too!) If you have an idea for how you want to handle this, do you want to propose (implement?) a specific PPISN mechanism?

ilyamandel commented 6 months ago

@SimonStevenson -- see also #272 for PPISNe; there I suggested two options: wind-like mass loss (relevant if mass is lost in many small pulsations) or a single episode of mass loss (max Blaauw kick). Still not clear what the natal kick should be, though -- what options would you suggest?

ilyamandel commented 6 months ago

Sorry, just re-read your comment above,

I'm fine with PPISN BHs having no kicks being the default assumption, but when I explicitly set an option to give black holes kicks I'd like the code to do what I asked.

So should we add a flag that would ask whether you want PPISN to be treated as normal CCSN for the purposes of natal kicks or to receive zero natal kicks (current default)?

ilyamandel commented 6 months ago

Addressed in #1123 , closing.