Home.md

@@ -61,14 +61,16 @@ To check the readiness of this pipeline for the analysis of O3 events.
 |[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.|
 |[LALInference.h](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInference.h) | [L882](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInference.h#L882) to [L883](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInference.h#L883)| Declaring above function LALInferencedQuadMonSdQuadMonA in header file|Abhirup & Anuradha G. |**Gupta**: looks good. <br> **Abhirup**: looks good.| No further changes made |
 |[LALInferenceTemplate.c](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c) | [L74](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L74) to [L76](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L76)| Adding extra params for sampling |Abhirup & Anuradha G. |**Gupta**: looks good. <br> **Abhirup**: looks good.| No further changes made. This is removed now as the spin-induced parameters are not part of testing-GR params |
-|[LALInferenceTemplate.c](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c) |[L358](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L358) to [L408](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L408) [L857](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L857) to [L905](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L905) [L1461](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L1461) to [L1512](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L1512)|Following ways of sampling are introduced. <br>1. If one parameter is passed among the four dQuadMon parameters, one would be able to sample on dQuadMon1, dQuadMon2, dQuadMonS or dQuadMonA individually.  <br> 2. For the two parameter case, either dQuadMon1 and dQuadMon2 or dQuadMonS and dQuadMonA can be sampled. If any other combination is passed, sampling will be done on dQuadMon1 and dQuadMon2 (dafault option) with a warning message. <br> 3. If more than two prameters are passed, sampling will be done on dQuadMon1 and dQuadMon2 (dafault option) with a warning message. |Abhirup & Anuradha G. |**Gupta**:Same set of lines in three palces? Also, in your implimentation you assumed \kappa_a to be zero. That means you assumed both the stars to have same \kappa.  How this assumption biases the results, especially on real data? <br> **Abhirup**: <br> The three sets of lines are for definitions inside 3 separate functions: LALInferenceROQWrapperForXLALSimInspiralChooseFDWaveformSequence, LALInferenceTemplateXLALSimInspiralChooseWaveform and LALInferenceTemplateXLALSimInspiralChooseWaveformPhaseInterpolated. <br> Wouldn't it be easier to allow sampling over dQuadMonA and if required fix it to injection or any other value from the config.ini using --pin-params = [dQuadMonA] or --fix-dQuadMonA [**ACTION ITEM**: Create a more generalised framework where any (and at most) two parameters can be sampled over; there will be an error if more than two parameters are specified to be sampled over; parameters can be held fixed from the config.ini if required] <br> **Krishnendu**: <br> 1. This has been implimented. <br> 2. We also did test runs for this for a high mass system and the run directory is here: LHO /home/nv.krishnendu/review_test_runs/Modified_template/. The corresponding .err and .out files contain error messages if there any. Right now, we are also printing the values of these parameters and these may be removed when we merge to the review branch.| The earlier functions (at three places) re-defined as the sampling parameter choices are revised now. |
+|[LALInferenceTemplate.c](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c) |[L358](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L358) to [L408](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L408) [L857](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L857) to [L905](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L905) [L1461](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L1461) to [L1512](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceTemplate.c#L1512)|Following ways of sampling are introduced. <br>1. If one parameter is passed among the four dQuadMon parameters, one would be able to sample on dQuadMon1, dQuadMon2, dQuadMonS or dQuadMonA individually.  <br> 2. For the two parameter case, either dQuadMon1 and dQuadMon2 or dQuadMonS and dQuadMonA can be sampled. If any other combination is passed, sampling will be done on dQuadMon1 and dQuadMon2 (dafault option) with a warning message. <br> 3. If more than two prameters are passed, the run exists. |Abhirup & Anuradha G. |**Gupta**:Same set of lines in three palces? Also, in your implimentation you assumed \kappa_a to be zero. That means you assumed both the stars to have same \kappa.  How this assumption biases the results, especially on real data? <br> **Abhirup**: <br> The three sets of lines are for definitions inside 3 separate functions: LALInferenceROQWrapperForXLALSimInspiralChooseFDWaveformSequence, LALInferenceTemplateXLALSimInspiralChooseWaveform and LALInferenceTemplateXLALSimInspiralChooseWaveformPhaseInterpolated. <br> Wouldn't it be easier to allow sampling over dQuadMonA and if required fix it to injection or any other value from the config.ini using --pin-params = [dQuadMonA] or --fix-dQuadMonA [**ACTION ITEM**: Create a more generalised framework where any (and at most) two parameters can be sampled over; there will be an error if more than two parameters are specified to be sampled over; parameters can be held fixed from the config.ini if required] <br> **Krishnendu**: <br> 1. This has been implimented. <br> 2. We also did test runs for this for a high mass system and the run directory is here: LHO /home/nv.krishnendu/review_test_runs/Modified_template/. The corresponding .err and .out files contain error messages if there any. Right now, we are also printing the values of these parameters and these may be removed when we merge to the review branch.| The earlier functions (at three places) re-defined as the sampling parameter choices are revised now. |
 |[LALInferenceInitCBC.c](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c) | [L2172](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L2172) to [L2175](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L2175)  [L2210](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L2210) to [L2250](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L2250)| Sampling options and warning (error) messages added inside the function `LALInferenceInitNonGRParams`| Abhirup & Anuradha G.| **Gupta**: L2172-L2179: Is prior till 20 large enough? L2214-L2218: looks good. <br> **Abhirup**: <br> 1. the current prior range excludes the space for boson stars <br>2.  tmpVal = 0, is at the default prior boundary <br> Perhaps making the prior more symmetric around zero addresses point 1 and keeps the starting value close to the middle of the prior. <br> **Krishnnendu**: <br> 1. The prior range has been increased to [-200, 200], but tmpVal = 0 is kept as it is.| The spin-induced parameters are removed from `LALInferenceInitNonGRParams(` and changed the following lines. <br> These params are added in `LALInferenceModel *LALInferenceInitCBCModel(LALInferenceRunState *state)` at [L793](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L793), the prior ranges are fixed at line [L880](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L880), following tidal parameter lines from [L1410](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L1410) to [L1425](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L1425) is changed.<br> As from the current changes, for the one parameter case any of the four parameters cane be sampled (one has to fix the other parameter using fix-parameters). <br> For the two parameter case, either {dQuadMon1, dQuadMon2} or {dQuadMonS, dQuadMonA}. <br> If one passes more than two parameters, the run will end with a exist message|
 |[cbcBayesCompPos.py](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/cbcBayesCompPos.py)  | [L59](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/cbcBayesCompPos.py#L59) and [L71](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/python/cbcBayesCompPos.py#L71)| `paramNameLatexMap` (convert parameter names to Latex), we define dQuadMon parameter as \delta\kappa <br> added to `clTableParams` (this is to add to the table of confidence levels)| Abhirup & Anuradha G.   |**Gupta**: Just \kappa_s is defined here, why not \kappa_1, \kappa_2, \kappa_a as in bayespputils.py? <br> **Abhirup**: Same question as Anuradha. <br> **Krishnnendu**: <br> 1. All the four parameters are added.   | -------- |
 |[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**
+## A detailed summary of revisions made to the review branch:
+> Commit-3: git hash: 8b1af14cba97cee8570201704c6917c42a89a97b. Separated the dDquadMon params from the testing GR params. Sampling options now allowd are; for the one parameter case any of the four parameters cane be sampled (one has to fix the other parameter using fix-parameters). <br> For the two parameter case, either {dQuadMon1, dQuadMon2} or {dQuadMonS, dQuadMonA}. <br> If one passes more than two parameters, the run will end with a exist message. Default prior on these parameters will be [-200, 200].
+> Commit-2: git hash: 95cc718a46e09c2f2eb7fe33090c8cfdc7aff468. Very generic sampling options are introduced. <br>1. If one parameter is passed among the four dQuadMon parameters, one would be able to sample on dQuadMon1, dQuadMon2, dQuadMonS or dQuadMonA individually.  <br> 2. For the two parameter case, either dQuadMon1 and dQuadMon2 or dQuadMonS and dQuadMonA can be sampled. If any other combination is passed, sampling will be done on dQuadMon1 and dQuadMon2 (dafault option) with a warning message. Mainly `lalinference/src/LALInferenceInitCBC.c` and `lalinference/src/LALInferenceTemplate.c` are changed.
+> Commit-1: git hash: 5157eea61128250568beca71edbc18c165a4a45e. This is the first commit. Main changes suggested by the reviewers to fix the sampling parameters choice related confusion and to fix the typoes in the plotting script.