| ... | @@ -66,8 +66,8 @@ Comments on the revised version |
... | @@ -66,8 +66,8 @@ Comments on the revised version |
|
|
- [ ] I would recommend changing the `Healyetal` fit name (e.g., [here](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/nrutils.py#L53)) to something more descriptive, likely involving the year, since there is also a Healy, Lousto, and Zlochower fit, and more than one Healy and Lousto fit (and possibly more such fits in the future). However, the naming can probably stay as is for the moment, unless David has strong feelings about this.
|
|
- [ ] I would recommend changing the `Healyetal` fit name (e.g., [here](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/nrutils.py#L53)) to something more descriptive, likely involving the year, since there is also a Healy, Lousto, and Zlochower fit, and more than one Healy and Lousto fit (and possibly more such fits in the future). However, the naming can probably stay as is for the moment, unless David has strong feelings about this.
|
|
|
|
|
|
|
|
* [conversions.py](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py)
|
|
* [conversions.py](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py)
|
|
|
- [ ] The check `elif "pan" in NRfit.lower()` [here](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L470) and elsewhere is not future-proof, since one could imagine another fit name having "pan" inside it. Checking for "panetal" would be much safer.
|
|
- [x] The check `elif "pan" in NRfit.lower()` [here](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L470) and elsewhere is not future-proof, since one could imagine another fit name having "pan" inside it. Checking for "panetal" would be much safer.
|
|
|
- [ ] The `_final_from_initial()` function's [documentation](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L395) should make it clear that in the precessing case it doesn't compute the final mass and spin the same way as in the waveform models (i.e., using the EOB fit evolution).
|
|
- [x] The `_final_from_initial()` function's [documentation](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L395) should make it clear that in the precessing case it doesn't compute the final mass and spin the same way as in the waveform models (i.e., using the EOB fit evolution).
|
|
|
|
|
|
|
|
* Minor:
|
|
* Minor:
|
|
|
- [ ] ["fit you wish to you"](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L460); also [here](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L496)
|
|
- [ ] ["fit you wish to you"](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L460); also [here](https://git.ligo.org/lscsoft/pesummary/-/blob/review_comments/pesummary/gw/file/conversions.py#L496)
|
| ... | | ... | |
