Allow sampling in aligned spin and spin magnitude
This MR will allow us to sample in the spin magnitude and the aligned component of the spin, rather than magnitude and tilt.
The reason to do this is that the aligned component of the spin is the best measured per component spin parameter. From preliminary tests, it looks like this will improve convergence with at least MultiNest.
Currently, this will only support the vanilla flat in magnitude, isotropic in orientation prior. It would take more effort to figure out the generic case.
This required making the CBCPriorDict a subclass of the ConditionalPriorDict, I know that people have reported seeing issues with this in the past, but I don't remember what they were. @moritz.huebner @rory-smith?
EDIT I think the issue with the conditional priors was the _check_if_prior_can_be_sampled method which I have removed. That was falling over for some conditions/constraints. I think the current version should be suitably robust.
Merge request reports
Activity
added Feature-request Priors + 1 deleted label
- Resolved by Colm Talbot
@colm.talbot I originally was against making the CBC prior dict conditional because I wasn't yet quite confident that it would always work. Especially, since we were going through some reviews at that stage. I'm happy for this to be added.
However, I can't quite guarantee that there aren't any downsides in terms of performance with the conditional prior dict compared to the regular prior dict if no conditions are used.
- Last reply by Colm Talbot
added Sampling label
-
Hi @moritz.huebner we decided on today's call that we would rather avoid having large changes to the code used in GW analyses between 1.0.2 and 1.0.3 as one of these versions are likely to be wanted for O3b analyses. Can we therefore push this to 1.0.4?
changed milestone to %Future
added 87 commits
-
6df7eb31...53aa1e5b - 86 commits from branch
master - 22e756ca - Merge branch 'master' into 'magnitude-aligned-spin-sampling'
-
6df7eb31...53aa1e5b - 86 commits from branch
added 7 commits
-
22e756ca...9201fde4 - 4 commits from branch
master - d1647abd - Merge branch 'master' into magnitude-aligned-spin-sampling
- aadc9be4 - Merge branch 'magnitude-aligned-spin-sampling' of git.ligo.org:Monash/tupak...
- ae8ee48f - Merge remote-tracking branch 'origin' into magnitude-aligned-spin-sampling
Toggle commit list-
22e756ca...9201fde4 - 4 commits from branch
added 1 commit
- 912fefc5 - Change how test of sampling from the prior works.
Resolved by Moritz HuebnerFYI: This change causes a recursion error at this line when combining results. I believe we saw this before with Soichiro's Mc-q prior, but I can't recall how it was fixed.
- Last reply by Moritz Huebner
added 28 commits
-
912fefc5...762568df - 25 commits from branch
master - 3854b319 - Merge remote-tracking branch 'origin/master' into magnitude-aligned-spin-sampling
- 5645d4e4 - Fix up documentation.
- 02fbf2d7 - Weaken prior equivalence tests to work with conditional priors.
Toggle commit list-
912fefc5...762568df - 25 commits from branch
added 7 commits
-
c7e6acb9...6681ce04 - 6 commits from branch
master - 245bcbbb - Merge branch 'master' into 'magnitude-aligned-spin-sampling'
-
c7e6acb9...6681ce04 - 6 commits from branch
added 3 commits
-
245bcbbb...81fca8e3 - 2 commits from branch
master - 011f3b11 - Merge branch 'master' into magnitude-aligned-spin-sampling
-
245bcbbb...81fca8e3 - 2 commits from branch
changed milestone to %1.1.0
mentioned in commit 13fbca39
mentioned in merge request !928 (merged)