Bugs in reweighing samples
Hi,
While trying to reweigh samples using the routine bilby.core.result.reweight
, I encountered two bugs (in bilby-v1.0.2
)
- In
bilby.core.result.get_weights_for_reweighting
, it assumed that the label for each row inresult.posterior
ranges from [0, len(result.posterior)-1] which is not necessarily the case especially rejection sampling was used prior to calling thisget_weights_for_reweighting
function. This will cause index-out-of-bound error. The simplest fix would be to change in L123 of bilby/core/result.py from
for ii, sample in result.posterior.iterrows():
to
for ii, (_, sample) in enumerate(result.posterior.iterrows()):
and also changing L174 of bilby/core/result.py from
return posterior[keep]
to
return posterior[keep].reset_index(drop=True)
- It seems that the action of doing
convert_to_flat_in_component_mass_prior
does not commute with doingbilby.core.result.get_weights_for_reweighting
. Doing reweighing before converting to flat in component mass prior yields no error but doing in the reverse order will yield an error saying that the key 'mass_1' is missing when the column actually exists. This is not desirable sinceconvert_to_flat_in_component_mass_prior
can be configured to run right after sampling, thus prohibiting reweighing of the samples in the future. This can be solved by properly assigning the component masses as one of the searched parameters and chirp mass and mass ratio being constrained in the result.search_parameter_keys and result.constraint_parameter_keys respectively. For example starting from L67 of bilby/gw/prior.py can be changed from
for key in ['chirp_mass', 'mass_ratio']:
priors[key] = Constraint(priors[key].minimum, priors[key].maximum, key, latex_label=priors[key].latex_label)
for key in ['mass_1', 'mass_2']:
priors[key] = Uniform(priors[key].minimum, priors[key].maximum, key, latex_label=priors[key].latex_label,
unit="$M_{\odot}$")
to
for key in ['chirp_mass', 'mass_ratio']:
priors[key] = Constraint(priors[key].minimum, priors[key].maximum, key, latex_label=priors[key].latex_label)
result.constraint_parameter_keys.append(key)
result.search_parameter_keys.remove(key)
for key in ['mass_1', 'mass_2']:
priors[key] = Uniform(priors[key].minimum, priors[key].maximum, key, latex_label=priors[key].latex_label,
unit="$M_{\odot}$")
result.search_parameter_keys.append(key)
result.constraint_parameter_keys.remove(key)
3.(?) Probably the function should be called reweigh
instead of reweight
but whatever
If people prefer, I can submit a merge request fixing issue 1 and 2 or it would be faster if the developers simply affect these simple changes.