TeamCOMPAS / COMPAS

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

Issue with mass transfer onto neutron star #1076

Open yuzhesong opened 3 months ago

yuzhesong commented 3 months ago

Describe the bug When attempting to fix the issues with neutron star recycling in the current version, @SimonStevenson and I have discovered that in some situations, the NS would accrete 0.25 Msol in one timestep of 1000 years, which is unphysical, since the accretion rate onto a neutron star should be Eddington limited.

@SimonStevenson and I have narrowed down the possible issue in the function void BaseBinaryStar::CalculateMassTransfer(), where m_FractionAccreted is calculated using CalculateMassAcceptanceRate().

It seems like that the code is calculating the mass transfer rate in this timestep described above as thermal mass loss rate, while it is much shorter than thermal time scale.

Label the issue

urgency_moderate - This is a moderately urgent issue. While this creates some issues, these cases are relatively rare and don't seem to pose problems to the majority of simulations.

To Reproduce I have attached a yaml file (in the format of txt as GitHub doesn't support uploading yaml file) that should be able to reproduce this case config_accretion_bug.txt

Versioning (please complete the following information):

SimonStevenson commented 2 months ago

I think I have a solution for this. I think the issue is that for these low-mass companion stars, the duration of the mass transfer should be long as the stars have long thermal timescales tau_th (tens of Myrs), which are much longer than the timestep dt we were taking during the mass transfer (in this case, the initial tilmestep is as small as dt = 0.001 Myr, since we are trying to resolve the early pulsar evolution).

To attempt to solve this, I have added a clause that if the star's thermal timescale is longer than the current timestep, we only accrete a fraction (dt / tau_th) of the mass that the donor star wants to donate.

This seems to have the desired effect of avoiding a large amount of mass being accreted in one timestep.

When I first did this, the system ended up merging after a few time steps. I managed to track this down to the tolerance ROOT_ABS_TOLERANCE used in MassLossToFitInsideRocheLobe, which by default is 1E-10. I noticed that the solution that was found was good to a few 1E-10. Relaxing the tolerance to 1E-9 solved this issue.

m1vtime m2vtime

ilyamandel commented 2 months ago

Hi @SimonStevenson , I think we speculated that this might be the issue during one of the code calls. However, it seems that the NS is actually doing the right thing: it is using the correct mass transfer rate (slow thermal timescale, not delta M / delta t (where the latter is the time step size) to decide that it can, in fact, accrete a sufficient fraction of mass while remaining sub-Eddington. Is that correct?

SimonStevenson commented 2 months ago

@ilyamandel that's right, with this fix, the system now evolves correctly. This will be part of @yuzhesong 's PR to fix MSP formation. Unfortunately, it seems that this issue (or one similar to it) persists even with this fix.

psr_mass1 psr_mass2

Here the different coloured lines refer to different values of alpha common envelope, which I am using as a proxy to manually adjust the post-common envelope orbital separation/orbital period. I looked at the relation between the final He WD mass and the final orbital period (at 14 Gyr) and compared this to the relation from Tauris & Savonije. It seems the relation is very different in COMPAS.

finalPorbMWD

Compare this to this figure from Tauris & Savonije:

Screen Shot 2024-04-22 at 4 41 13 pm

ilyamandel commented 2 months ago

OK, I think there are a couple of things here, @SimonStevenson :

SimonStevenson commented 2 months ago

1) Well there's a big difference between a system that transfers 0.25 Msol in 1000 years and one that transfers a similar amount of mass in ~600 Myr. Not least of which is that it was causing problems for our pulsar recycling code, but any attempt to post process these systems for LMXB/MSP studies will result in the wrong answers for lifetimes/birthtimes etc. Switching between main sequence models here should be ~fine, I think.

2) Yes they do include those, and for close binaries (with Porb < 1 day) those effects can certainly become important, but for the wide binaries with Porb >> 1 day studied in Tauris & Savonije, they should be mostly negligible (and they say that in their paper). So all that should be important here is the nuclear evolution of the low-mass companion star that drives the mass transfer (which should be ok in COMPAS) and the details of mass transfer.

ilyamandel commented 2 months ago

OK, fair point on 1. Then we'll need to think about how to deal with MT more broadly, if we want to split up things like, say, the fast phase of case A MT over multiple time steps...

On 2, I take your point, but then don't understand how their binary shrinks on mass transfer from a low mass WD to a high mass NS. One can analytically confirm that both fully conservative and fully non-conservative (isotropic re-emission from accretor) MT should widen such binaries. Do they assume very large angular momentum carried away by non-accreted mass?

SimonStevenson commented 2 months ago

They make the standard assumption of isotropic re-emission from the accreting neutron star. The binaries do widen. They start with orbits of a few days and end with orbits of 2--1000 days. Sorry if the plot above was confusing, that's not an evolutionary track, but a locus connecting the final evolutionary states for a series of different initial configurations. That can be seen more clearly in this figure:

Screen Shot 2024-04-29 at 5 54 03 pm
SimonStevenson commented 2 months ago

I ran a similar experiment with BSE and it seems to get closer to the Tauris & Savonije result, though still not quite the same relation. BSE_MWD_Porb

ilyamandel commented 2 months ago

I think it would be easier to understand what goes wrong by looking at the evolution of a single binary rather than the end-point of a whole population.

In principle, even when we are transferring a lot of mass in one step, we do split up the transferred mass into lots of small pieces in BaseBinaryStar::CalculateMassTransferOrbit(), and the code there looks sensible -- but it would be really good to understand if we are doing something wrong!

ilyamandel commented 1 month ago

@SimonStevenson, @yuzhesong : OK, I checked the code for mass transfer, and I don't see any evidence of bugs -- it seems to be doing what was intended. So, the question is whether the intent was correct. :)

There are two places where one might quibble with the code assumptions.

  1. We assume a fixed mass transfer rate for the entire episode as computed in a given time step, which is pre-computed at the start of the timestep. That means both donor and accretor properties are considered only at the start of the timestep, even if we donate, say, the entire envelope of an evolved star on a single timestep. Note that we correctly integrate the orbital evolution by taking small substeps, but the accretion efficiency beta is determined once and for all. Given that we are using the thermal timescale for evolved stars, I expect this might change the donor's nominal thermal timescale (and hence, accretion efficiency) by a factor of ~2 [R is roughly constant at the RL size, L is constant, but M can ~halve]. However, given that the accretor is changing even more drastically and our treatment is an over-simplification regardless (see, e.g., Lau+ 2024 on Hamstars, which we should eventually move to using), I don't think this is the key issue.

  2. A larger issue for mass transfer from MS stars onto compact objects is that we are using the donor's thermal timescale to estimate mass transfer efficiency, comparing it with the accretor's (Eddington-limited, in this case) accretion timescale. That generally leads to extremely non-conservative mass transfer. However, the nuclear can be much longer than the thermal timescale for MS stars, particularly low-mass MS stars, i.e., closer to the accretion timescale, and hence may allow for much slower mass transfer. I suspect this is the also responsible for #889 .

I propose that we leave choice 1 as is, but work on choice 2.

ilyamandel commented 1 month ago

OK, I have a plane for how to allow for nuclear timescale mass transfer without taking too much of a hit on computational cost, will submit a PR either today or Monday (offline for the weekend) and ask you and Jeff to review.

ilyamandel commented 1 month ago

Should be at least partially addressed in #1123 , which allows MS stars to donate mass on their nuclear timescale (see above). @SimonStevenson , @yuzhesong , let me know how things go once that's pulled in (not closing for now since I haven't tested consequences specifically for NS accretion).

ilyamandel commented 1 month ago

Sorry, I spoke prematurely, and there were still significant problems with case A mass transfer following the previous PR. Please try #1132 instead (once it's reviewed and pulled in).

ilyamandel commented 1 month ago

Somehow, the relevant change didn't seem to make it into #1132 (mysteries of git?), but should be there in #1133, so please use that instead, once approved.

ilyamandel commented 1 month ago

@yuzhesong , @SimonStevenson -- have you been able to check if this issue is now resolved? So far, I seem to find that the MT is behaving much better than previously in all of the tests I've considered, including accretion onto compact objects.

yuzhesong commented 1 month ago

@ilyamandel your fix in #1133 does indeed let us have long case A mass transfer, but it seems like the mass transfer rate is too low. I made a comparison between your fix and including your fix and Simon's treatment in the first figure below. mass

This seems to indicate that NS won't accrete enough mass to let it reach to the typical MSP magnetic field strength at around 10e8 Gauss, shown below.

image

And Figs. 2a, 2b, 2c & 2d in this Tauris et al 1999 paper (https://ui.adsabs.harvard.edu/abs/1999A%26A...350..928T/abstract) also seems to indicate a higher mass transfer rate.

We will run some MESA models for some more insights.

ilyamandel commented 1 month ago

@yuzhesong : The nuclear timescale of a 0.9 Msol star is more than 10^10 years. That means I don't expect it to transfer mass faster than 0.9 Msol / 10^10 years, or 10^{-10} Msol per year = 10^{-4} Msol / Myr.

The Mdot plot you made shows that we currently estimate it as being a bit over 10^{-5} Msol / Myr, which I consider sensible (we are using the radial expansion timescale, which is lower than 10^10 years except late on the MS).

I am not sure how @SimonStevenson arrived at 5e-3 Msol/Myr, but that seems to me to be rather high [if this is taken from BSE, I think BSE rates are artificial because they depend on the amount by which the star exceeds the Roche lobe, which just depends on the size of the timestep].

Do you have a physically motivated alternative prescription for nuclear timescale mass transfer? We can either replace the current one, or create an alternative option, which would certainly be in the spirit of rapid pop synth. Alternatively, we can also parametrise our ignorance by including a new option as a multiplier to the existing nuclear timescale mass transfer rate.

ilyamandel commented 2 weeks ago

Aside: correction to MT in #1147, please use that for further testing.

yuzhesong commented 2 weeks ago

@ilyamandel noted, will test and if everything works OK, then we'll merge the MSP branch into dev.