Skip to content

Changes

Page history
Update Home authored by Anuradha Gupta's avatar Anuradha Gupta
Show whitespace changes
Inline Side-by-side
Home.md
View page @ 37d7f842
......@@ -65,7 +65,7 @@ To check the readiness of this pipeline for the analysis of O3 events.
|[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.| 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, 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. <br> **new comments** <br> **Gupta:** looks good.| The earlier functions (at three places) re-defined as the sampling parameter choices are revised now. The new line numbers are [L358](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceTemplate.c#L358) to [L378](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceTemplate.c#L378), [L831](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceTemplate.c#L831) to [L851](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceTemplate.c#L851) and [L1415](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceTemplate.c#L1415) to [L1435](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceTemplate.c#L1435) |
|[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. <br> **new comments** <br> **Gupta:** looks good. <br> 99bcd8666d6e279ebcad988219ba56c4e70cef50 <br> **Gupta:** Why [this line](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L1432) is showing up in your commit?| 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|
|[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. <br> **new comments** <br> **Gupta:** looks good. <br> 99bcd8666d6e279ebcad988219ba56c4e70cef50 <br> **Gupta:** Why [this line](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L1432) is showing up in your commit? <br> I am not sure if [this statement](https://git.ligo.org/nv.krishnendu/lalsuite/blob/Ks_21March2019/lalinference/src/LALInferenceInitCBC.c#L1453-1454) is necessary.| 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|
|[LALInferenceProposal.c](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceProposal.c) | [L98](https://git.ligo.org/nv.krishnendu/lalsuite/blob/08e05a526d3e2bca780bf4fa08fa83df85209d51/lalinference/src/LALInferenceProposal.c#L98) | added to the list of intrinsic params |Abhirup & Anuradha G. | **Gupta:** minor issue, "dQuadMonS" can be put before "dQuadMonA" <br> 99bcd8666d6e279ebcad988219ba56c4e70cef50 <br> **Gupta:** looks good.| |
|[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. <br> **new comments** <br> **Gupta:** looks good. | -------- |
|[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. <br> **new comments** <br> **Gupta:** looks good.| -------- |
......