step_SN mass check; force manual calculation if not matching

[maxbriel]

This is a temporary fix for several issues:

1. Incorrect masses BH. #303
2. NS/WD NaN masses #280

The previous code had the following issue: The check for SN_type and CO_type would not recalculate the masses from the interpolator. Thus, giving a NaN or a wrong remnant mass from a different CO_type class.

Now, it forces all mismatching SN_type and CO_types to be fully recalculated with the given remnant mass prescriptions based on the preSN star. This calculation is the same as if use_interp_values=False or for stars evolved with step_detached or step_disrupted.

[celiotine]

Ok I have some sad news. It seems like this error is occurring for binaries where the SN type and object type do actually match, so this fix is not catching everything. Here is an example problem binary where the SN type is ECSN and the object type is NS:

star_1 = SingleStar(**{'mass': 7.1125623905943485, 'state': 'H-rich_Core_H_burning',\
                'natal_kick_array': [19.068527695032785, 3.5633775052142633, 1.2378597315236688, 2.4627960384344374]})
star_2 = SingleStar(**{'mass': 5.041150596272908, 'state': 'H-rich_Core_H_burning',\
                'natal_kick_array': [None, None, None, None]})

binary = BinaryStar(star_1, star_2,
                   **{'time': 0.0, 
                      'state': 'detached', 
                      'event': 'ZAMS',
                      'orbital_period': 35.66303442178471, 
                      'eccentricity': 0.0},
                    properties = sim_prop)

Evolving this binary throws the usual error resulting from NaN values:

It looks like this is happening for most combinations of SN types and object types (e.g. CCSN/NS, CCSN/BH, etc.) For the temp fix we can explicitly check if the interpolated values are NaN regardless of mismatch, but it seems the deeper issue may be different than what we thought?

For binaries that do have a mismatch, the interpolated values are sometimes NaN and sometimes not:

[celiotine]

My most recent commit adds an explicit check if the interpolated values are NaN (rn just checks if the mass is NaN). This is fixing the issue for now, I am not getting any more errors as I reported above.

[tassos25]

@celiotine will coordinate with @ZepeiX and @philipp-rajah to decide if this PR is required on top of #325

[maxbriel]

Just to have it recorded: Even with retrained interpolators, the non-match situation still occurs and, thus, we should catch these situations and process them. A warning is thrown as well to inform the user that something went wrong, but it will try to continue with the step_SN.

[ZepeiX]

Regarding the mismatch between SN_type and CO_type, is that only 'WD' and 'NS'? I have some concerns about whether we should turn to use 'profiles' in this case. Not profiles actually, but the interpolated core properties. Because it might be a problem of the interpolation of SN_type not the CO properties, which could probably be fixed in the future.

[maxbriel]

The issue is not only with WD's and NS's, but I've also seen discrepancies between BHs and NSs at their boundary.

I think the main question is: what interpolator/classifier do you trust more? The SN_type or the CO_type? And if there's a mismatch do we still trust the CO_interpolation_class values?

This is similar to the already present check and warning is the model does not match up: https://github.com/POSYDON-code/POSYDON/blob/c7ecd73ef4d5b05345cf9f762cf057f1251a3c6e/posydon/binary_evol/SN/step_SN.py#L524

[maxbriel]

@ZepeiX and I had a short discussion on this and I dug a bit on why the discrepancy occurs.

The discrepancy between SN_type and CO_type originated from them being trained on different classifiers. SN_type was trained on the mass transfer classification, while the CO_type was trained on its own classes. See the code how it used to be: https://github.com/POSYDON-code/POSYDON/blame/3ca9243d3a2b459a5a9d4ca341c611e1365d0582/bin/run-pipeline#L342-L353

The CO_type got changed to CO_interpolation_class, but CO_type and SN_type were now both trained within the mass transfer classes. It's unclear how sparse this could become, possibly resulting in more issues with the values.

With PR #325, this makes CO_type and SN_type be trained per CO_interpolation_class. This should also remove discrepancies between the CO_type and SN_type. However, @ZepeiX has noticed a binary "going out of hull" and giving a CO_type and SN_type discrepancy. This indicates another potential issue.

I think a fallback, as introduced here, would still be useful in such rare cases. Since a warning is given, this is clear to the user something happened, although I've got no strong opinions.