Changes

Page history
Update Reviewer Notes authored by ABHIRUP GHOSH's avatar ABHIRUP GHOSH
Show whitespace changes
Inline Side-by-side
Reviewer-Notes.md
View page @ 06f9847b
...@@ -6,200 +6,8 @@ The overall plan for this review is here: ...@@ -6,200 +6,8 @@ The overall plan for this review is here:
<img src="uploads/31ec35108885b5852e0a80eeaffe301a/PNG_image.png" height="300"> <img src="uploads/31ec35108885b5852e0a80eeaffe301a/PNG_image.png" height="300">
# Code Review # Code Review
A detailed report on the code review is [here](https://git.ligo.org/nv.krishnendu/lalsuite/wikis/Code-review).
## Build details # Results Review
A detailed report on the results review is [here](https://git.ligo.org/nv.krishnendu/lalsuite/wikis/Results-review).
| Branch | Git hash | Diff Patch | Build status | \ No newline at end of file
|--------|----------|------------|--------------|
| [Ks_21March2019](https://git.ligo.org/nv.krishnendu/lalsuite/commits/Ks_21March2019) | [7ec73781](https://git.ligo.org/nv.krishnendu/lalsuite/commit/7ec7378129a05b343dcf5d088f97426e41321949) | [diff.patch](https://git.ligo.org/nv.krishnendu/lalsuite/blob/7ec7378129a05b343dcf5d088f97426e41321949/diff.patch) | SUCCESSFUL |
## Codes
### `lalinference/python/cbcBayesCompPos.py`
| Tasks| Status |
|---|---|
| Comparison of GR case between master and feature branch | |
| Comparison of all 4 non-GR cases | |
### `lalinference/python/cbcBayesPPAnalysis.py`
| Tasks| Status |
|---|---|
| Cases that cover all 4 non-GR parameters | |
### `lalinference/python/cbcBayesPostProc.py` and `lalinference/python/lalinference/bayespputils.py`
| Tasks| Status |
|---|---|
| Posteriors on all 4 non-GR parameters | |
| Priors on all 4 non-GR parameters | |
### `lalinference/src/LALInference.c` and `lalinference/src/LALInference.h`
| Tasks| Status |
|---|---|
| Output of `LALInferencedQuadMonSdQuadMonA` function| |
We use the following python script to check the output of the `LALInferencedQuadMonSdQuadMonA` function. We generate samples from a distribution uniform in dQuadMonS and dQuadMonA, and plot the scatter of the corresponding dQuadMon1, dQuadMon2 values.
```
dQuadMonS = np.random.uniform(-200,200,10000)
dQuadMonA = np.random.uniform(-200,200,10000)
dQuadMon1, dQuadMon2 = [],[]
for idx in range(10000):
x = li.dQuadMonSdQuadMonA(dQuadMonS[idx], dQuadMonA[idx])
dQuadMon1.append(x[0]), dQuadMon2.append(x[1])
plt.figure()
plt.scatter(dQuadMonS, dQuadMonA, color='r', alpha=0.1, label='S-A')
plt.scatter(dQuadMon1, dQuadMon2, color='k', alpha=0.05, label='1-2')
plt.legend(loc='best')
plt.xlabel('$\delta \kappa _S$')
plt.ylabel('$\delta \kappa _A$')
```
![download](uploads/be779d3e2272a0e6737d4233d7cca2e8/download.png)
### `lalinference/src/LALInferenceInitCBC.c`/`lalinference/src/LALInferenceProposal.c`/`lalinference/src/LALInferenceTemplate.c`
Different sampling scenarios:
| Sampling case| Options to be passed |
|---|---|
| SA | --dQuadMonSA --dQuadMonS-min WW --dQuadMonS-max XX --dQuadMonA-min YY --dQuadMonA-max ZZ |
| S | --dQuadMonSA --fix-dQuadMonA 0.0 --dQuadMonS-min WW --dQuadMonS-max XX |
| S | --dQuadMonSA --fix-dQuadMonS 0.0 --dQuadMonA-min WW --dQuadMonA-max XX |
| 12 | --dQuadMon12 --dQuadMon1-min WW --dQuadMon1-max XX --dQuadMon2-min YY --dQuadMon2-max ZZ |
| 1 | --dQuadMon12 --fix-dQuadMon2 0.0 --dQuadMon1-min WW --dQuadMon1-max XX |
| 2 | --dQuadMon12 --fix-dQuadMon1 0.0 --dQuadMon2-min WW --dQuadMon2-max XX |
| | Tasks| Status |
|---|---|---|
| 1. | Priors on all 4 non-GR parameters | |
| 2. | fix-params should fix the posterior at tmp-value | |
| 3. | `fprintf(stdout,"Both options (from 12 and SA) are given . . Exiting . .\n");` | |
| 4. | `fprintf(stdout,"Only one option (from 12 and SA) is given . . Working fine . .\n");` | |
| 5. | `fprintf(stdout,"Both dQuadMonS and dQaudMonA are sampled");` | |
| 6. | `fprintf(stdout,"dQM1: %e, dQM2: %e, dQMS: %e, dQMA: %e \n",dQuadMon1,dQuadMon2,dQuadMonS,dQuadMonA);`| |
| 7. | `fprintf(stdout,"Both dQuadMon1 and dQaudMon2 are sampled");`||
| 8. | `fprintf(stdout,"dQM1: %e, dQM2: %e \n",dQuadMon1,dQuadMon2);` | |
Reference Injection file: [S190814bv.xml](uploads/1e189529dbc83ded65aac7a1bfca84a2/S190814bv.xml)
#### Task 3
LALInference Command and output:
```
lalinference_nest --L1-flow 20 --approx IMRPhenomPv2threePointFivePN --psdlength 492.0 --V1-timeslide 0 --V1-cache LALSimAdVirgo --chirpmass-max 9.519249 --nlive 1024 --inj S190814bv.xml --comp-max 54.40873232560802 --adapt-temps --srate 4096.0 --event 0 --H1-timeslide 0 --V1-fhigh 135.811 --neff 1000 --seglen 16.0 --L1-channel L1:LDAS-STRAIN --L1-fhigh 135.811 --stance-max 500 --0noise --trigtime 900000000 --tol 1.0 --psdstart 899999474.0 --H1-cache LALSimAdLIGO --progress --tolerance 0.1 --H1-channel H1:LDAS-STRAIN --V1-channel V1:h_16384Hz --comp-min 1.6378358668107575 --resume --V1-flow 20 --fref 20 --outfile lalinferencenest-0-V1H1L1-900000000.0-0.hdf5 --L1-cache LALSimAdLIGO --randomseed 1732024500 --dataseed 3000 --L1-timeslide 0 --q-min 0.055728090000841224 --chirpmass-min 5.141979 --H1-flow 20 --H1-fhigh 135.11 --ifo V1 --ifo H1 --ifo L1 --dQuadMon12 --dQuadMonSA
Both options (from 12 and SA) are given . . Exiting . .
Error: cannot use more than one of --dQuadMon12 and --dQuadMonSA.
XLAL Error - LALInferenceInitCBCModel (LALInferenceInitCBC.c:1451): Invalid argument
Segmentation fault: 11
```
### Task 4-8
**--dQuadMon12**
LALInference Command and output:
```
lalinference_nest --L1-flow 20 --approx IMRPhenomPv2threePointFivePN --psdlength 492.0 --V1-timeslide 0 --V1-cache LALSimAdVirgo --chirpmass-max 9.519249 --nlive 1024 --inj S190814bv.xml --comp-max 54.40873232560802 --adapt-temps --srate 4096.0 --event 0 --H1-timeslide 0 --V1-fhigh 135.811 --neff 1000 --seglen 16.0 --L1-channel L1:LDAS-STRAIN --L1-fhigh 135.811 --stance-max 500 --0noise --trigtime 900000000 --tol 1.0 --psdstart 899999474.0 --H1-cache LALSimAdLIGO --progress --tolerance 0.1 --H1-channel H1:LDAS-STRAIN --V1-channel V1:h_16384Hz --comp-min 1.6378358668107575 --resume --V1-flow 20 --fref 20 --outfile lalinferencenest-0-V1H1L1-900000000.0-0.hdf5 --L1-cache LALSimAdLIGO --randomseed 1732024500 --dataseed 3000 --L1-timeslide 0 --q-min 0.055728090000841224 --chirpmass-min 5.141979 --H1-flow 20 --H1-fhigh 135.11 --ifo V1 --ifo H1 --ifo L1 --dQuadMon12
Read end time 900000000.000000
Using detector-based sky frame
Only one option (from 12 and SA) is given . . Working fine . .
--- ---
Templates will run with precessing spins
Templates will run using Approximant 72 (IMRPhenomPv2), phase order 7, amp order -1, spin order -1 tidal order -1 in the frequency domain.
--- ---
Template function called is "LALInferenceTemplateXLALSimInspiralChooseWaveform"
set tolerance.
Sprinkling 1024 live points, may take some time
Both dQuadMon1 and dQaudMon2 are sampleddQM1: 1.506288e+02, dQM2: -1.427024e+02
Both dQuadMon1 and dQaudMon2 are sampleddQM1: -4.831145e+01, dQM2: -5.314133e+01
Both dQuadMon1 and dQaudMon2 are sampleddQM1: -9.102365e+01, dQM2: -7.010662e+01
Both dQuadMon1 and dQaudMon2 are sampleddQM1: -9.118437e+00, dQM2: -2.130467e+01
```
**--dQuadMonSA**
LALInference Command and output:
```
Using end time from injection file: 900000000.000000
Read end time 900000000.000000
Using detector-based sky frame
Only one option (from 12 and SA) is given . . Working fine . .
--- ---
Templates will run with precessing spins
Templates will run using Approximant 72 (IMRPhenomPv2), phase order 7, amp order -1, spin order -1 tidal order -1 in the frequency domain.
--- ---
Template function called is "LALInferenceTemplateXLALSimInspiralChooseWaveform"
set tolerance.
Sprinkling 1024 live points, may take some time
Both dQuadMonS and dQaudMonA are sampleddQM1: 7.926428e+00, dQM2: 2.933312e+02, dQMS: 1.506288e+02, dQMA: -1.427024e+02
Both dQuadMonS and dQaudMonA are sampleddQM1: -1.014528e+02, dQM2: 4.829881e+00, dQMS: -4.831145e+01, dQMA: -5.314133e+01
Both dQuadMonS and dQaudMonA are sampleddQM1: -1.611303e+02, dQM2: -2.091703e+01, dQMS: -9.102365e+01, dQMA: -7.010662e+01
Both dQuadMonS and dQaudMonA are sampleddQM1: -3.042310e+01, dQM2: 1.218623e+01, dQMS: -9.118437e+00, dQMA: -2.130467e+01
```
### Comments:
* Ask about new line character.
* The case where we fix one of the parameters, does indeed print fixed values for that parameter during the sampling.
```
lalinference_nest --L1-flow 20 --approx IMRPhenomPv2threePointFivePN --psdlength 492.0 --V1-timeslide 0 --V1-cache LALSimAdVirgo --chirpmass-max 9.519249 --nlive 1024 --inj S190814bv.xml --comp-max 54.40873232560802 --adapt-temps --srate 4096.0 --event 0 --H1-timeslide 0 --V1-fhigh 135.811 --neff 1000 --seglen 16.0 --L1-channel L1:LDAS-STRAIN --L1-fhigh 135.811 --stance-max 500 --0noise --trigtime 900000000 --tol 1.0 --psdstart 899999474.0 --H1-cache LALSimAdLIGO --progress --tolerance 0.1 --H1-channel H1:LDAS-STRAIN --V1-channel V1:h_16384Hz --comp-min 1.6378358668107575 --resume --V1-flow 20 --fref 20 --outfile lalinferencenest-0-V1H1L1-900000000.0-0.hdf5 --L1-cache LALSimAdLIGO --randomseed 1732024500 --dataseed 3000 --L1-timeslide 0 --q-min 0.055728090000841224 --chirpmass-min 5.141979 --H1-flow 20 --H1-fhigh 135.11 --ifo V1 --ifo H1 --ifo L1 --dQuadMon12 --fix-dQuadMon1 0.0
Read end time 900000000.000000
Using detector-based sky frame
Only one option (from 12 and SA) is given . . Working fine . .
--- ---
Templates will run with precessing spins
Templates will run using Approximant 72 (IMRPhenomPv2), phase order 7, amp order -1, spin order -1 tidal order -1 in the frequency domain.
--- ---
Template function called is "LALInferenceTemplateXLALSimInspiralChooseWaveform"
set tolerance.
Sprinkling 1024 live points, may take some time
Both dQuadMon1 and dQaudMon2 are sampleddQM1: 0.000000e+00, dQM2: -1.259211e+02
Both dQuadMon1 and dQaudMon2 are sampleddQM1: 0.000000e+00, dQM2: -4.098038e+01
Both dQuadMon1 and dQaudMon2 are sampleddQM1: 0.000000e+00, dQM2: -7.584580e+01
```
## Overall code status
| Code | Test | Status |
|------|------|--------|
| `lalinference/python/cbcBayesCompPos.py` | 1. | |
| `lalinference/python/cbcBayesPPAnalysis.py` | | |
| `lalinference/python/cbcBayesPostProc.py `| | |
| `lalinference/python/lalinference/bayespputils.py` | | |
| `lalinference/src/LALInference.c` | | |
| `lalinference/src/LALInference.h` | | |
| `lalinference/src/LALInferenceInitCBC.c` | | |
| `lalinference/src/LALInferenceProposal.c` | . | . |
| `lalinference/src/LALInferenceTemplate.c` | | |
### Notes on the codes
* There seems to be an inconsistency between naming the variables with lower and upper case: "dquadmona" vs "dQuadMon1". The post-processing scripts have lower case, while the sampling scripts have upper case. `Clarify with Krishnendu`.
\ No newline at end of file