Remnant-fits.md

@@ -10,14 +10,18 @@ The conversions are handled in by the [`pesummary.gw.file.conversions` module](h
 
 *  inspected version: ed801a91
 *  [nrutils wrappers](https://git.ligo.org/lscsoft/pesummary/-/blob/master/pesummary/gw/file/nrutils.py):
-* [ ]  I only find the two functions that use `args[0]` etc a bit dangerous. (`_bbh_final_spin_precessing_using_non_precessing_fit` and `_bbh_final_spin_precessing_projected`)
+* [x]  I only find the two functions that use `args[0]` etc a bit dangerous. (`_bbh_final_spin_precessing_using_non_precessing_fit` and `_bbh_final_spin_precessing_projected`)
 However, continuing to read in [conversions.py](https://git.ligo.org/lscsoft/pesummary/-/blob/master/pesummary/gw/file/conversions.py), it seems passing things as positional arguments is the overall design, and can't be easily fixed without global redesign, so this is ok.
+    > I have changed the code to make the arguments explicit rather than args[0]. This does not solve the problem in its entirety as this would require, as you say, a global redesign.
 
 * [conversions.py](https://git.ligo.org/lscsoft/pesummary/-/blob/master/pesummary/gw/file/conversions.py)
 * [ ]  `final_mass_of_merger_from_NR` and `final_mass_of_merger` take `final_spin` arguments (presumably for the Healy et al fits), but don't pass them on. Since these extra arguments are only used in nrutils for minor speedup, this is probably not a problem, but it looks odd; maybe just drop them from your local interface?
-* [ ]  Why do `final_mass_of_merger_from_NR` and `final_spin_of_merger_from_NR`, when not in the "average" case, always call the respective `non_precessing` functions?
-* [ ] `_final_mass_of_merger` seems to be missing the tilt arguments...?
-* [ ] If those arguments to `_final_mass_of_merger` are changed, then probably in L1254f the param lists checking needs to be updated.
+* [x]  Why do `final_mass_of_merger_from_NR` and `final_spin_of_merger_from_NR`, when not in the "average" case, always call the respective `non_precessing` functions?
+    > For the `final_mass_of_merger_from_NR`, the only available fits are `non_precessing` so that is why it only called `non_precessing` fits. I have corrected the `final_spin_of_merger_from_NR` to allow for precessing fits. This is done by prepending the `NRfit` by `precessing_`.
+* [x] `_final_mass_of_merger` seems to be missing the tilt arguments...?
+    > Tilt angles are not required for PESummary because `spin_1z` and `spin_2z` are already precomputed and can be passed directly.
+* [x] If those arguments to `_final_mass_of_merger` are changed, then probably in L1254f the param lists checking needs to be updated.
+    > See previous comment.
 * [ ] There is this check `if all(i in self.parameters for i in final_params):` for calling `_final_spin_of_merger`. So what happens if the tilts are missing? No final spin computed at all? Shouldn't there be a non-precessing fallback?
 
 * Higher level comments /  looking ahead: