... | @@ -61,7 +61,7 @@ To check the readiness of this pipeline for the analysis of O3 events. |
... | @@ -61,7 +61,7 @@ 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]. |
|
|
|[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]. |
|
|
|[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.|
|
|
|[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.|
|
|
|[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.|
|
|
|[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.|
|
|
|[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/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L358) to [L373](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L373) [L825](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L825) to [L840](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L840) [L1397](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L1397) to [L1412](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L1412)| Sampling is done on dQuadMonS keeping dQuadMonA to zero|Abhirup & Anuradha G. |**Gupta**:Same set of lines in three palces? [Answer: They are defined under 3 separate functions: LALInferenceROQWrapperForXLALSimInspiralChooseFDWaveformSequence, LALInferenceTemplateXLALSimInspiralChooseWaveform and LALInferenceTemplateXLALSimInspiralChooseWaveformPhaseInterpolated]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? |
|
|
|[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/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L358) to [L373](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L373) [L825](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L825) to [L840](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L840) [L1397](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L1397) to [L1412](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceTemplate.c#L1412)| Sampling is done on dQuadMonS keeping dQuadMonA to zero|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|
|
|
|[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/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2172) to [L2179](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2179) [L2214](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2214) to [L2218](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2218)| Function to calculate dQuadMonS and dQuadMonA from dQaudMon1 and dQuadMon2| 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> 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.
|
|
|[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/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2172) to [L2179](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2179) [L2214](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2214) to [L2218](https://git.ligo.org/nv.krishnendu/lalsuite/blob/eb5f2658c960ad689625c66c7df9e9373a4a27d6/lalinference/src/LALInferenceInitCBC.c#L2218)| Function to calculate dQuadMonS and dQuadMonA from dQaudMon1 and dQuadMon2| 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> 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.
|
|
|[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)| | Abhirup & Anuradha G. |**Gupta**: Just \kappa_s is defined here, why not \kappa_1, \kappa_2, \kappa_a as in bayespputils.py? |
|
|
|[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)| | Abhirup & Anuradha G. |**Gupta**: Just \kappa_s is defined here, why not \kappa_1, \kappa_2, \kappa_a as in bayespputils.py? |
|
|
|[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)| | 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. |
|
|
|[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)| | 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. |
|
... | | ... | |