... | ... | @@ -56,8 +56,6 @@ To check the readiness of this pipeline for the analysis of O3 events. |
|
|
|
|
|
- Git hash of the branch to be reviewed: 5157eea61128250568beca71edbc18c165a4a45e
|
|
|
|
|
|
> This for the first review meeting on 24/05/2019, 07/06/2019 and 21/06/2019. The modifications suggested by the reviewers are in this commit:
|
|
|
|
|
|
| Files modified |Lines changed| Short explanation | Person Responsible | Reviewer Response | Addressing Issue (if any) |
|
|
|
| -------- | --- | ----- | -------- | -------- | -------- |
|
|
|
|[LALInference.c](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInference.c) | [L2322](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInference.c#L2322) to [L2327](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInference.c#L2327)| Function to calculate dQuadMonS and dQuadMonA from dQaudMon1 and dQuadMon2|Abhirup & Anuradha G. |**Gupta**: looks good. <br> **Abhirup**: looks good. <br> General clarifications: <br>1. Out of (dQuadMonS, dQuadMonA,dQaudMon1,dQuadMon2) only two are independent; what are the problems of sampling over just two and constructing the others in post-processing, rather than keeping all four as sampling parameters [**Answer**: there are certain combinations of these quantities that are easier to sample over to yield better constraints; provide options to either sample on (dQaudMon1,dQuadMon2) or (dQuadMonS, dQuadMonA) depending on user.] <br>2. And if all 4 parameters need to be kept in the code, is there a way of making it explicit to the user to choose at most 2 of the 4 parameters for sampling [**Action Item**: Error message along the lines of tidal parameters to make sure that sampling happens over at most 2 parameters]. | <br> **Krishnendu**: <br> 1. These comments are addressed. The current structure provides options to either sample on (dQaudMon1,dQuadMon2) or (dQuadMonS, dQuadMonA). If more than two parameters are passed, the run will not proceed further and exist with an error message.|
|
... | ... | @@ -69,6 +67,9 @@ To check the readiness of this pipeline for the analysis of O3 events. |
|
|
|[cbcBayesPPAnalysis.py](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/cbcBayesPPAnalysis.py) | [L88](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/cbcBayesPPAnalysis.py#L88)|Added all four parameters to the list of `posterior_name_to_latex_name` | Abhirup & Anuradha G.|**Gupta**: \kappa_1 and \kappa_2 are also defined here but not \kappa_a. Is this because \kappa_a is always zero in your implimentation? But \kappa_a is defined in bayespputils.py, why? Also, there is a typo, dquadmon2-->dquadmon1. <br> **Abhirup**: Following up on Anuradha's comment, it would be good to keep the implementation as generalised as possible, which means, adding all four parameters in the post-processing codes, wherever possible. <br> **Krishnnendu**: <br> 1. All the four dQuadMon parameters are added and typos are fixed. | -------- |
|
|
|
|[bayespputils.py](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/lalinference/bayespputils.py) | [L148](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/lalinference/bayespputils.py#L148) and [L484](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/lalinference/bayespputils.py#L484)| Created a list `spin_induced_quad_terms` and added this to `strongFieldParams` <br> Created corresponding plot labels inside `plot_label(param)` |Abhirup & Anuradha G. |**Gupta**: looks good. <br> **Abhirup**: looks good.| -------- |
|
|
|
|
|
|
- A detailed summary of revisions made to the review branch:
|
|
|
<br> **Commit-1**
|
|
|
|
|
|
|
|
|
|
|
|
# Tasks/tests to be accomplished
|
... | ... | |