[enhancement] HMS+HMS Mergers rejuvenation according to Schneider+2016#788
[enhancement] HMS+HMS Mergers rejuvenation according to Schneider+2016#788ezapartas wants to merge 15 commits into
Conversation
…d the possibility to match with total_mass_h1
for more information, see https://pre-commit.ci
…to 'Glebbeek+2013'
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
With @dimsour94 we tested it with various options run. |
added the options and default values of of rel_mass_lost_HMS_HMS and HMS_HMS_merging_rejuvenation in the .ini file of the population.
|
We should merge PR#785 into 4/23/26: I went ahead and merged v2.3 (which contains PR#785) into this. |
|
We should update the docs here with these changes. 4/24/26: I added some updates to the docs |
for more information, see https://pre-commit.ci
… options in merger step
for more information, see https://pre-commit.ci
|
|
||
| merged_star.mass = (star_base.mass + comp.mass) * (1.-rel_mass_lost_HMS_HMS) | ||
|
|
||
| X_average_merged = (star_base.total_mass_h1 + comp.total_mass_h1 - (1.-rel_mass_lost_HMS_HMS) * X_initial) / merged_star.mass |
There was a problem hiding this comment.
Are you sure these equations are correct?
The
This is the same with the Y_average_merged.
| X_average_merged = (star_base.total_mass_h1 + comp.total_mass_h1 - (1.-rel_mass_lost_HMS_HMS) * X_initial) / merged_star.mass | |
| X_average_merged = (star_base.total_mass_h1 + comp.total_mass_h1 - (rel_mass_lost_HMS_HMS *(star_base.mass + comp.mass)* X_initial)) / merged_star.mass |
There was a problem hiding this comment.
@maxbriel I think you are right. Thanks for catching this! @dimsour94 can you confirm?
| mass_weight2='mass')) | ||
|
|
||
| # The change of masses occurs after the calculation of weighted averages | ||
| merged_star.mass = (star_base.mass + comp.mass) * (1.-self.rel_mass_lost_HMS_HMS) |
There was a problem hiding this comment.
This line needs to be edited. It uses self.rel_mass_lost_HMS_HMS, which is set to a string in the rejuvenated case. This will fail.
Either move this inside the else statement or make sure it's compatible with the rejuvenated case.
| # first we calculate the mass loss percentage: | ||
| if self.rel_mass_lost_HMS_HMS == "Glebbeek+2013": | ||
| # Eq. 4 of Glebbeek+2013 | ||
| rel_mass_lost_HMS_HMS = (0.3 * q) / ((1.0 + q) ** 2.0) |
There was a problem hiding this comment.
If rel_mass_lost_HMS_HMS is set to a string, self.rel_mass_lost_HMS_HMS will also become a string. Since you're using rel_mass_lost_HMS_HMS variable later without the self, it's okay, but the original code uses and expects self.rel_mass_lost_HMS_HMS as a float. This should be edited to account for!
| merged_star.mass = (star_base.mass + comp.mass) * (1.-rel_mass_lost_HMS_HMS) | ||
|
|
||
| X_average_merged = (star_base.total_mass_h1 + comp.total_mass_h1 - (1.-rel_mass_lost_HMS_HMS) * X_initial) / merged_star.mass | ||
| Y_average_merged = (star_base.total_mass_he4 + comp.total_mass_he4 - (1.-rel_mass_lost_HMS_HMS) * Y_initial) / merged_star.mass |
There was a problem hiding this comment.
Same issue as with X_average_merged
| merged_star.center_he4 = Y_average_merged | ||
| merged_star.surface_he4 = Y_average_merged | ||
|
|
||
| for key in STARPROPERTIES: |
There was a problem hiding this comment.
What would you like to happen to the surface abundances: " c12", "n14", and "o16"?
|
|
||
| # The change of masses occurs after the calculation of weighted averages | ||
| merged_star.mass = (star_base.mass + comp.mass) * (1.-self.rel_mass_lost_HMS_HMS) | ||
| merged_star.total_mass_h1 = merged_star.center_h1 * merged_star.mass |
There was a problem hiding this comment.
You've already set the total_mass_h1 in the rejuvenated case. This will always overwrite that value here. Should this be moved inside the else block?
| # Not used? | ||
| #X_average1 = star_base.total_mass_h1 / star_base.mass | ||
| #X_average2 = comp.total_mass_h1 / comp.mass |
| merged_star.surface_he4 = Y_average_merged | ||
|
|
||
| for key in STARPROPERTIES: | ||
| # these stellar attributes become np.nan |
There was a problem hiding this comment.
Shift comment to correct indentation
| # these stellar attributes become np.nan | |
| # these stellar attributes become np.nan |
| #X_average1 = star_base.total_mass_h1 / star_base.mass | ||
| #X_average2 = comp.total_mass_h1 / comp.mass | ||
|
|
||
| Z_div_Zsun_init = star_base.metallicity |
There was a problem hiding this comment.
Are there other variables you want/need to check that they exists?
|
@ezapartas @dimsour94 I've reviewed the PR and you will need to verify a few things in the implementation, because it currently brakes some of the other evolution. Furthermore, it seems that the average abundances equations are not the same as in Schneider+2016. We can probably work through it with/for you, if you're okay with the suggested changes. Currently causes binary candidates to fail in our dev-tools suite. |
|
In the old test suite, binary 4 fails. This is probably due to the change from float to a string for on |
HMS_HMS_merging_rejuvenation = Trueto stronger rejuvenation of MS+MS mergers (according to Glebbeek+2023, Schneider+2016, following the recent Wang+2025 too). So it should be considered a change in POSYDON results for few types mergers-This PR includes already the changes in #785 and builds on it. If accepted, that PR becomes unnecessary