Resolve #430 (Add normalisation flag to constrained prior)
This MR resolves issue #430 (closed) to properly normalize ConstrainedPriorDict.prob() when sampling the prior. This MR allows for users to input a normalisation flag when creating an instance of a Constraint prior and when calling ConstrainedPriorDict.prob() if there are normalisation constants given will return the normalisation*prob and otherwise use the ratio of total samples to kept samples as this normalisation constant. Pass normalisation=1 when creating a Constraint Prior for behaviour before this MR.
Merge request reports
Activity
added Priors label
Resolved by Bruce EdelmanSorry for ignoring this for such a long time.
I'm becoming decreasingly enamoured with the analytic renormalising. There's really a new normalisation for every possible combination of parameters.
If there's a constraint on mass ratio, the marginal probability for inclination doesn't change.
I think a neat solution might be to have a cached normalisation which checks:
- the parameters we're evaluating
probfor - the constraint parameters
The reason for caching is that working out the normalisations is going to be reasonably expensive.
The appropriate normalisations can be worked out by drawing a bunch of samples for each of the parameters we're interested in and then seeing which satisfy the constraints.
Does this sound sensbile?
- the parameters we're evaluating
- Last reply by Bruce Edelman
added 25 commits
-
6a90fc3c...4436ec59 - 24 commits from branch
lscsoft:master - c04966e3 - Merge branch 'master' of https://git.ligo.org/lscsoft/bilby into constraint
-
6a90fc3c...4436ec59 - 24 commits from branch
added 1 commit
- 6579fa7a - fixed flake8 issues, and check sample and outsample for constraint keys
added 8 commits
-
6579fa7a...6ee08a83 - 6 commits from branch
lscsoft:master - c11f406b - Merge branch 'master' of https://git.ligo.org/lscsoft/bilby into constraint
- 41f729a7 - fixed constraint checking logic
-
6579fa7a...6ee08a83 - 6 commits from branch
added 1 commit
- 8d94cdd3 - added check for minimum samples in constraint nomralizationa nd propoerlly...
added 1 commit
- 83ff1129 - revert to match master formatting in base.py
- Resolved by Colm Talbot
@colm.talbot, I am having trouble with the caching as implemented here. To cache the ratio the arguments which are the list of keys to evaluate prob over, and self, both need to be hashable to be used as keys in the cache dict. However, python dicts are not hashable, so it does not like having reference to self(PriorDict) in the cached method. I found a workaround to make a hashable dictionary here https://stackoverflow.com/questions/1151658/python-hashable-dicts but it does mention that this will not work if the dictionary needs to be mutable after being set as a key for the cache.
Do you have ideas on this and maybe the best way to cache this? The necessary keys for our caching dict need to include the details of the PriorDict of course since different bounds on parameters will affect this. This is also complicated since the cached method uses other methods of the PriorDict..
Edited by Bruce Edelman - Last reply by Bruce Edelman
added 2 commits
added 1 commit
- 1b781bd0 - changed lru_cache to be functools not methodtools version
added 1 commit
- 5b1a5778 - change from lru_cached to just a plain dictionary approach for ease of application
added 1 commit
- d1a295c4 - changed from repr to tuple to convert list to something that can be used as a...