MAINT: tweaks to prior constraints

[mj-will]

Some tweaks to prior contraints

• Raise an error if a constraint is present but the parameter key is missing. Currently, it's possible to have constraints that are silently ignored, e.g. because the conversion function doesn't generate them.
• Remove an unreachable if statement. Prior to Change probability initialization to use np.ones_like #1028, evaluate_constraints returned 1.0 if there were no valid constraints and there was a check for this where the normalization was set to 1. I've removed this for now. Alternatively we could return 1.0 if all values are true but the while should do this anyway.

[mj-will]

I think we may need to have an option where this doesn't raise an error since it is called in sample_subset_constrained and there are cases there where it shouldn't fail. E.g. if evaluating the usual GW prior and the masses aren't present.

[ColmTalbot]

I think we may need to have an option where this doesn't raise an error since it is called in sample_subset_constrained and there are cases there where it shouldn't fail. E.g. if evaluating the usual GW prior and the masses aren't present.

We could add this to the CBCPriorDict?

[mj-will]

I think we may need to have an option where this doesn't raise an error since it is called in sample_subset_constrained and there are cases there where it shouldn't fail. E.g. if evaluating the usual GW prior and the masses aren't present.

We could add this to the CBCPriorDict?

I've added strict option, let me know what you think.

[ColmTalbot]

It looks like this is a hard condition on the sample, prob, and ln_prob methods.
I don't know that the latter two are correctly handled.
For example, the following is currently valid, but I suspect will fail under this change.
Is that right?

import bilby

priors = bilby.gw.prior.BBHPriorDict
priors.ln_prob(dict(a_1=0.3, tilt_1=0.7))

[ColmTalbot]

Why has this been removed?

[mj-will]

My understanding was that this was a hacky check for if there aren't any constraints. Prior to #1028, evaluate_constraints would return 1.0 if there weren't any constraints, so this would skip the rest of this function.

After the changes 1028, that only happens if the sampling_chunk=1 is, so it's not clear to me this actually serves it's intended purpose.

Looking at this again, I think the lines above this may be redundant as well and this whole thing could be replaced with a has_constraints (or similar) property.

[mj-will]

It looks like this is a hard condition on the sample, prob, and ln_prob methods. I don't know that the latter two are correctly handled. For example, the following is currently valid, but I suspect will fail under this change. Is that right?

import bilby

priors = bilby.gw.prior.BBHPriorDict
priors.ln_prob(dict(a_1=0.3, tilt_1=0.7))

Yes, this fails but I think it would fail before as well since the conversion function would fail with a KeyError due to the masses missing.

You raise a good point though, I had assumed ln_prob should only called with a complete sample but I don't know if that's actually the case.