Additional-Review-(marginalization-over-joint-population+EoS-uncertainty).md

@@ -30,9 +30,11 @@ for each component mass, meaning that the total contribution to the weights shou
 
 The new version of the code relies on the user to specify the draw probability, so this is rendered moot. Within the tests described below, this was implemented correctly within [this script](https://git.ligo.org/reed.essick/mmax-model-selection/-/blob/gw-distributions/test/o3-nsbh/src/convert-event-csv#L48). **Update link once merge request is accepted**
 
-- [X] Reviewer signoff on implementation in example
-- [ ] Reviewer signoff that in new version of the code, implementation of Jacobians should be done through inputs, not on this code base.
-    - I cannot yet confirm this because I can't find the `mmms.io` module.
+- [X] Reviewer signoff on implementation in example (AMF 28 August 2023)
+- [X] Reviewer signoff that in new version of the code, implementation of Jacobians should be done through inputs, not on this code base. (AMF 28 August 2023)
+    - I cannot yet confirm this because I can't find the `mmms.io` module (28 August 2023)
+    - `mmms.io` was added by Reed on 28 August 2023, and I have now confirmed that this version of the code does not implement an on-the-fly addition of prior columns and instead requires them to be fed in by the user. 
+    - I do recommend adding a helpstring to the prior column name argument saying that the prior needs to be in terms of source frame masses, since all of the distributions allowed by this code (and implemented in `gwdistributions` are in parameterized in terms of source frame masses. It can just have a warning that if the population is parameterized in terms of luminosity distance, so should the prior, and same for redshift. However I do not require this for review signoff since it is purely cosmetic. I have left more details in comments in the MR.
 
 **bad population reweighing**
 
@@ -73,8 +75,8 @@ The correct sum is implemented within the updated code [here](https://git.ligo.o
 This difference can cause discrepencies between the old and the new code when there is a nontrivial marginalization over the population (i.e., when there is more than a single population sample `\Lambda_p`).
 These can be much larger than the statistical uncertainty from the finite number of Monte Carlo samples (see [below](#comparison-to-previous-results)), but have never been large enough to change the scientific conclusions drawn. Furthermore, [ApJ 915 L5 (2021)](https://iopscience.iop.org/article/10.3847/2041-8213/ac082e) only reported results assuming a fixed population (flat in source-frame component masses) and therefore this issue did not affect the published results.
 
-- [X] Reviewer signoff on desired expression (within technical note)
-- [X] Reviewer signoff on implementation within the code
+- [X] Reviewer signoff on desired expression within technical note (AMF 28 August 2023)
+- [X] Reviewer signoff on implementation within the code (AMF 28 August 2023)
      -  The code is normalized w.r.t. the single event parameters ($\theta$ or `\theta_i$ above)
 
 
@@ -100,7 +102,7 @@ In brief, I find
   * excellent agreement when the mass, spin, redshift distributions are exactly known (i.e., no marginalization over population parameters)
   * statistically significant (but scientifically inconsequental) differences when we additionally marginalize over uncertainty in the population. As described [above](#identified-bugs-and-associated-fixes), this is to be expected because of bugs in the previous version of the code.
 
-- [X] Reviewer signoff on the above statements
+- [X] Reviewer signoff on the above statements (AMF 28 August 2023)
 
 
 **GW200105 C01_PHMcombined_highspin**