search

ColmTalbot / gwpopulation

CPU/GPU agnostic gravitational-wave population inference
https://colmtalbot.github.io/gwpopulation/
MIT License
31 stars 46 forks source link

added change to vt.py to use mu rather than marginalized pdet #63

Closed jacobgolomb closed 1 year ago

jacobgolomb commented 1 year ago

This PR changes the default call of the ResamplingVT to use the estimator of the Monte Carlo rather than the marginalized pdet from Farr 2019. Justification: see top of pg 14 on Essick & Farr for example.

jacobgolomb commented 1 year ago

I think the pipelines fail because I changed how pdet is computed so we won't get the same likelihood that the pipeline wants to see.

ColmTalbot commented 1 year ago

Also, you should just change the likelihood value in the failing test as we are deliberately changing how the likelihood is calculated.

jacobgolomb commented 1 year ago

I would also suggest just always returning mu when not applying the correction.

To clarify, you are suggesting we remove the n_eff convergence check here? I'd be wary to do this as I could imagine the sampler oversampling regions of low injection coverage, unless this PR is merged along with the ability to cut on total likelihood uncertainty (which may be the plan, just checking).