COSMIC-PopSynth / COSMIC

COSMIC (Compact Object Synthesis and Monte Carlo Investigation Code)
GNU General Public License v3.0
45 stars 58 forks source link

Implementing Eddington limit for BHs based on spinup/ISCO #570

Closed michaelzevin closed 1 year ago

michaelzevin commented 1 year ago

This PR adjusts the Eddington limit for BHs based on the implementation in Marchant et al. 2017, following Equations 3-5. It introduces a new global parameter Mbh_initial that remembers the initial mass of the BH at formation, and calculates the eta parameter that alters the mass transfer rate based on where the ISCO is, assuming the BH is spinning up from stable accretion as in Thorne 1970.

As of now, I added an additional global parameter BHbirth_ind that is 0 before the BH forms and 1 after so that Mbh_initial doesn't get overwritten in hrdiag.f, though there's probably a more glamorous way to ensure this.

This actually causes the Eddington limit for BHs to be ~1 order of magnitude higher for low-spinning BHs than the standard BSE prescription!!! It is unclear where Hurley got the expression for the Eddington limit that is implemented in stock BSE (Equation 67 of Hurley et al. 2002). The two expressions are identical for eta = 0.5, which is slightly above the maximum value of eta=0.42 which is attained for a maximally-spinning BH (one that accreted more than sqrt(6) times its initial mass).

Attached are the mass transfer rates for a few binaries, comparing the current BH accretion limit to the one in this PR. There is still an issue of deltam_1 for the first timestep written into the bcm array not obeying the imposed Eddington limit, which can be seen in a few of these systems.

eddlim

There are probably other changes that need to be made to the unit test files, so let me know what else needs to be done!

codecov[bot] commented 1 year ago

Codecov Report

Merging #570 (538753a) into develop (37a19db) will increase coverage by 0.99%. The diff coverage is 18.75%.

@@             Coverage Diff             @@
##           develop     #570      +/-   ##
===========================================
+ Coverage    87.13%   88.12%   +0.99%     
===========================================
  Files           40       40              
  Lines        25393    25432      +39     
===========================================
+ Hits         22124    22410     +286     
+ Misses        3269     3022     -247     
Impacted Files Coverage Δ
cosmic/src/hrdiag.f 39.24% <0.00%> (-0.30%) :arrow_down:
cosmic/src/evolv2.f 57.63% <23.08%> (-0.44%) :arrow_down:
cosmic/sample/cmc/elson.py 88.24% <0.00%> (-0.65%) :arrow_down:
cosmic/bse_utils/zdata.py 100.00% <0.00%> (ø)
cosmic/bse_utils/zcnsts.py 100.00% <0.00%> (ø)
cosmic/evolve.py 84.58% <0.00%> (+29.17%) :arrow_up:
cosmic/sample/sampler/multidim.py 86.49% <0.00%> (+68.92%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

michaelzevin commented 1 year ago

@katiebreivik I've done what you suggested. Looks like the unit tests never encountered super-Eddington accretion so they still pass.

There's still the issue with the deltam1_bcm for the first timestep of the BCM array sometimes still being super-Eddington, though I don't think this is a huge issue.

katiebreivik commented 1 year ago

Hey @michaelzevin sorry for being slow! -- can you open an issue with an example binary that still goes super Eddington on the first time step? I just want to make sure we don't lose it in the mix. Once that issue's open, feel free to merge!

michaelzevin commented 1 year ago

Good call. I've opened an Issue at #571. I'll now go ahead and merge.