bilby issueshttps://git.ligo.org/lscsoft/bilby/-/issues2024-03-25T15:30:48Zhttps://git.ligo.org/lscsoft/bilby/-/issues/737Transitioning to sampler plugins2024-03-25T15:30:48ZMichael Williamsmichael.williams@ligo.orgTransitioning to sampler pluginsI've opened this issue to track changes that should be made as we migrate samplers out of the main code and into their own packages. Hopefully others can edit this as they identify things.
**Samplers**
We haven't agreed a policy for mo...I've opened this issue to track changes that should be made as we migrate samplers out of the main code and into their own packages. Hopefully others can edit this as they identify things.
**Samplers**
We haven't agreed a policy for moving the samplers, but I believe nessai will be the first, followed by pypolychord. As we decided on more samplers, we can add them here.
In order of decreasing priority:
- [ ] nessai
- [ ] pypolychord
**Examples**
Some examples may need updating because of the changes
- [ ] `examples/core_examples/alternative_samplers/linear_regression_pymc_custom_likelihood.py` (should be made if/when moving pymc)https://git.ligo.org/lscsoft/bilby/-/issues/730Follow-up from "Draft: Add support for sampler plugins"2024-02-23T15:30:45ZMichael Williamsmichael.williams@ligo.orgFollow-up from "Draft: Add support for sampler plugins"The following discussion from !1299 should be addressed:
- [x] @michael.williams started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/1299#note_928209): (+1 comment)
> Thinking about this in connection with b...The following discussion from !1299 should be addressed:
- [x] @michael.williams started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/1299#note_928209): (+1 comment)
> Thinking about this in connection with bilby_pipe, specifically the logic here: https://git.ligo.org/lscsoft/bilby_pipe/-/blob/master/bilby_pipe/job_creation/nodes/analysis_node.py#L125. As it stands, this can prevent samplers from being used out-of-the-box with bilby pipe unless you set `transfer-files=False`.
>
> What would folks think about adding one or two attributes to the sampler classes that specify the files and/or directories the sampler will create? Something like:
>
> ```
> expected_output_files = []
> expected_output_directories = []
> ```
> or a function that returns them based on the labels.
>
> bilby_pipe could then use these attributes to create the directories and placeholder files, rather than having to hard code things. It would then fall on the sampler plugins to maintain this rather than the bilby_pipe maintainers
>
> I'm happy to implement something like this in a separate MR if there's interest.Michael Williamsmichael.williams@ligo.orgMichael Williamsmichael.williams@ligo.orghttps://git.ligo.org/lscsoft/bilby/-/issues/722ptemcee parallelisation issue2024-01-29T05:15:48ZColm Talbotcolm.talbot@ligo.orgptemcee parallelisation issueA downstream user reported an issue with ptemcee on MacOS with parallelization.
I found that it works with the multiprocessing start method set to `fork`, which is a temporary workaround.
As a fix, we need to make sure the [`_sampling_...A downstream user reported an issue with ptemcee on MacOS with parallelization.
I found that it works with the multiprocessing start method set to `fork`, which is a temporary workaround.
As a fix, we need to make sure the [`_sampling_convenience_dump`](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/base_sampler.py#L47) works with the [`LikePriorEvaluator`](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/ptemcee.py#L1406).
An initial solution that seemed to work was to update the `LikePriorEvaluator` as
```python
def __init__(self):
from .base_sampler import _sampling_convenience_dump
self.periodic_set = False
self.dump = _sampling_convenience_dump
```
and then using `self.dump` rather than `_sampling_convenience_dump` throughout.
This might cause issues if `emcee/kombine/zeus` pass the `LikePriorEvaluator` through the multiprocessing pool.
See https://github.com/bandframework/Taweret/issues/53https://git.ligo.org/lscsoft/bilby/-/issues/716Time to remove polychord?2023-11-10T16:01:57ZColm Talbotcolm.talbot@ligo.orgTime to remove polychord?After reworking the test suite, we're still having [issues](https://git.ligo.org/lscsoft/bilby/-/jobs/3025662) with installing polychord because it has to be built from source.
I suggest that we begin the process of removing polychord a...After reworking the test suite, we're still having [issues](https://git.ligo.org/lscsoft/bilby/-/jobs/3025662) with installing polychord because it has to be built from source.
I suggest that we begin the process of removing polychord as a supported sampler. With the following process:
- remove it from the test suite ASAP.
- remove it from the package at the next major/minor release (presumably 2.3).
This would be the first time we've dropped support for a sampler, but I think it would be good to formalize that any supported sampler should be available from conda-forge.
Thoughts @gregory.ashton @michael.williams @sylvia.biscoveanu?https://git.ligo.org/lscsoft/bilby/-/issues/710Dynesty sampler unexpectedly closing all plots2023-11-09T21:08:29ZGitLab Support BotDynesty sampler unexpectedly closing all plotsI've noticed when I run bilby with either the dynesty or dynamic dynesty samplers, any plots I had open before calling run_sampler are closed. I think this may be caused by a few calls to matplotlib closing all figures (plt.close('all'))...I've noticed when I run bilby with either the dynesty or dynamic dynesty samplers, any plots I had open before calling run_sampler are closed. I think this may be caused by a few calls to matplotlib closing all figures (plt.close('all')) in the plot_current_state() function of the dynesty sampler. Would it be possible to change these so that they only close the figures created by running bilby, and not any other plots opened by other parts of a script?
Thanks!https://git.ligo.org/lscsoft/bilby/-/issues/690AttributeError when calling bilby.run_sampler with Dynesty sampler2024-03-15T21:54:34ZBrian HealyAttributeError when calling bilby.run_sampler with Dynesty samplerHello, I'm trying to run this code (https://github.com/nuclear-multimessenger-astronomy/nmma/blob/main/nmma/em/analysis.py) that uses bilby. When the code calls `bilby.run_sampler`, I get the following error having to do with the `Dynest...Hello, I'm trying to run this code (https://github.com/nuclear-multimessenger-astronomy/nmma/blob/main/nmma/em/analysis.py) that uses bilby. When the code calls `bilby.run_sampler`, I get the following error having to do with the `Dynesty` object that bilby creates:
```
Traceback (most recent call last):
File "/Users/bhealy/miniforge3/envs/nmma_api2/bin/light_curve_analysis", line 33, in <module>
sys.exit(load_entry_point('nmma==0.0.8', 'console_scripts', 'light_curve_analysis')())
File "/Users/bhealy/nmma/nmma/em/analysis.py", line 608, in main
result = bilby.run_sampler(
File "/Users/bhealy/miniforge3/envs/nmma_api2/lib/python3.9/site-packages/bilby/core/sampler/__init__.py", line 190, in run_sampler
sampler = sampler_class(
File "/Users/bhealy/miniforge3/envs/nmma_api2/lib/python3.9/site-packages/bilby/core/sampler/dynesty.py", line 234, in __init__
int(check_point_delta_t / self._log_likelihood_eval_time / 10), 10
AttributeError: 'Dynesty' object has no attribute '_log_likelihood_eval_time'
```
I'm happy to provide more details as needed. Thank you!Alexandre GoettelAlexandre Goettelhttps://git.ligo.org/lscsoft/bilby/-/issues/687Reproducilbity of dynesty results2024-02-14T15:05:15ZMichael Williamsmichael.williams@ligo.orgReproducilbity of dynesty resultsIn the current release of bilby, it is not clear to me that dynesty runs are 100% reproducible. This came up in the asimov review when two runs with functionally identical bilby_pipe ini files, run with the same environment, had differen...In the current release of bilby, it is not clear to me that dynesty runs are 100% reproducible. This came up in the asimov review when two runs with functionally identical bilby_pipe ini files, run with the same environment, had different results.
In the example below, I set the numpy random seed and passed the sampling seed kwargs to `run_sampler` (which is removed for dynesty) and I get different evidence values. Changing the sampler to nessai gives the same evidence values.
Is there something else we should be setting for dynesty to ensure the run is seeded? The [changelog](https://github.com/joshspeagle/dynesty/blob/master/CHANGELOG.md#120---2022-03-31) implies that we should be setting a random state, but I can't see that in the bilby wrapper.
## Example
```python
import bilby
import numpy as np
np.random.seed(1234)
data = np.random.normal(3, 4, 100)
class SimpleGaussianLikelihood(bilby.Likelihood):
def __init__(self, data):
"""
A very simple Gaussian likelihood
Parameters
----------
data: array_like
The data to analyse
"""
super().__init__(parameters={"mu": None, "sigma": None})
self.data = data
self.N = len(data)
def log_likelihood(self):
mu = self.parameters["mu"]
sigma = self.parameters["sigma"]
res = self.data - mu
return -0.5 * (
np.sum((res / sigma) ** 2) + self.N * np.log(2 * np.pi * sigma**2)
)
likelihood = SimpleGaussianLikelihood(data)
priors = dict(
mu=bilby.core.prior.Uniform(0, 5, "mu"),
sigma=bilby.core.prior.Uniform(0, 10, "sigma"),
)
np.random.seed(1234)
label = "gaussian_example_0"
outdir = "outdir"
result = bilby.run_sampler(
likelihood=likelihood,
priors=priors,
sampler="dynesty",
nlive=100,
outdir=outdir,
label=label,
sampling_seed=1234,
)
np.random.seed(1234)
label = "gaussian_example_1"
outdir = "outdir"
result = bilby.run_sampler(
likelihood=likelihood,
priors=priors,
sampler="dynesty",
nlive=100,
outdir=outdir,
label=label,
sampling_seed=1234,
)
```https://git.ligo.org/lscsoft/bilby/-/issues/677Sampling time is incorrect after checkpointing2023-03-20T15:03:14ZMichael Williamsmichael.williams@ligo.orgSampling time is incorrect after checkpointingThe sampling time reported in the bilby result file is incorrect for `nessai` if the run has been checkpointed and resumed. Instead of reporting the total time, it reports the time since the sampler was resumed. This may affect other sam...The sampling time reported in the bilby result file is incorrect for `nessai` if the run has been checkpointed and resumed. Instead of reporting the total time, it reports the time since the sampler was resumed. This may affect other samplers, but `dynesty` and `bilby_mcmc` both have workarounds and correctly record the time
**Note:** `nessai` stores the sampling time in its own result object and this time does accurately track the time after resuming.https://git.ligo.org/lscsoft/bilby/-/issues/676Some MCMC proposals with non-trivial boundaries breaks detailed balance2023-03-22T02:18:38ZColm Talbotcolm.talbot@ligo.orgSome MCMC proposals with non-trivial boundaries breaks detailed balance@gregory.ashton noticed that some periodic parameters return biased results when using the density estimate proposals.
This is because we don't track the correct jacobian term that takes into account the wrapping/folding for this and oth...@gregory.ashton noticed that some periodic parameters return biased results when using the density estimate proposals.
This is because we don't track the correct jacobian term that takes into account the wrapping/folding for this and other proposals that have non-trivial jacobians.
I think the simplest fix would be to only call the `apply_boundaries` method if the `log_jacobian == 0`.
Correctly tracking the full density would be a combinatoric nightmare in large numbers of dimensions with arbitrary combinations of boundaries.https://git.ligo.org/lscsoft/bilby/-/issues/644Bilby MCMC: Sampler gets stuck when checkpoint fails2022-09-29T09:48:18ZColm Talbotcolm.talbot@ligo.orgBilby MCMC: Sampler gets stuck when checkpoint failsI noticed that one of my jobs that was failing to checkpoint and it got stuck in a loop where it tried to write the checkpoint at every iteration. I think the issue is that if the checkpoint this line always fails.
I think the fix will ...I noticed that one of my jobs that was failing to checkpoint and it got stuck in a loop where it tried to write the checkpoint at every iteration. I think the issue is that if the checkpoint this line always fails.
I think the fix will be to set `self.check_point_delta_t = np.inf` in [this clause](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/bilby_mcmc/sampler.py#L388).https://git.ligo.org/lscsoft/bilby/-/issues/613Conversion function not applied to posterior2022-01-18T15:07:20ZMichael Williamsmichael.williams@ligo.orgConversion function not applied to posteriorWhilst running `nessai` for the O4 PE MDC I noticed that with the latest version of `bilby` the conversion function specified in `run_sampler` is not being applied to the posterior before it is saved.
It seems that this is caused by the...Whilst running `nessai` for the O4 PE MDC I noticed that with the latest version of `bilby` the conversion function specified in `run_sampler` is not being applied to the posterior before it is saved.
It seems that this is caused by the changes introduced in https://git.ligo.org/lscsoft/bilby/-/merge_requests/958, in particular the addition of the check for an existing posterior [here](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/__init__.py#L270) which skips the conversion. Previously this check happened within `samples_to_posterior` and the conversion function was always applied.
As far as I can tell this only affects samplers that manually convert their output to the posterior dataframe instead of letting conversion happen [here](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/__init__.py#L270). So `cpnest` and `nessai`.
I think the easiest fix is to just tweak the interface for both samplers and have them add `result.samples` instead of `result.posterior` but I'm keen to hear people's thoughts since there may be other cases where this issue comes up.Michael Williamsmichael.williams@ligo.orgMichael Williamsmichael.williams@ligo.orghttps://git.ligo.org/lscsoft/bilby/-/issues/601Update `dynesty` API to version 1.1+2022-12-06T00:07:32ZGregory Ashtongregory.ashton@ligo.orgUpdate `dynesty` API to version 1.1+In #599 and #600 issues where noted between bilby and the new `dynesty` API. We need to update the API to `dynesty` to utilise the new version.In #599 and #600 issues where noted between bilby and the new `dynesty` API. We need to update the API to `dynesty` to utilise the new version.https://git.ligo.org/lscsoft/bilby/-/issues/565Sample from the prior test failing due to timeout2021-04-20T10:49:04ZGregory Ashtongregory.ashton@ligo.orgSample from the prior test failing due to timeoutThe sample from the prior test is failing due to a timeout in the CI. It was removed in !949 to enable development, this issues tracks re-instating it (and fixing it for the nightly tests).The sample from the prior test is failing due to a timeout in the CI. It was removed in !949 to enable development, this issues tracks re-instating it (and fixing it for the nightly tests).https://git.ligo.org/lscsoft/bilby/-/issues/559check_draw doesn't catch nan log_likelihood values.2021-06-03T01:34:59ZEamonn O'Sheacheck_draw doesn't catch nan log_likelihood values.I've been getting dynesty errors from nan loglikelihoods during construction of the Dynesty sampler object. I was stepping through the bilby internals with pdb and was finding that the nan loglikelihoods were happening in `get_initial_po...I've been getting dynesty errors from nan loglikelihoods during construction of the Dynesty sampler object. I was stepping through the bilby internals with pdb and was finding that the nan loglikelihoods were happening in `get_initial_points_from_prior` in the `base_sampler.py`. I was confused as to why the function `check_draw` wasn't preventing `theta`'s which produce nan loglikelihoods from being added to the returned list. I believe the reason is a bug in `check_draw`, which has the following implementation.
```
def check_draw(self, theta, warning=True):
bad_values = [np.inf, np.nan_to_num(np.inf), np.nan]
if abs(self.log_prior(theta)) in bad_values:
if warning:
logger.warning('Prior draw {} has inf prior'.format(theta))
return False
if abs(self.log_likelihood(theta)) in bad_values:
if warning:
logger.warning('Prior draw {} has inf likelihood'.format(theta))
return False
return True
```
However, nan's don't behave as other values with respect to equality. `nan != nan` is `True`, so if `self.log_likelihood(theta)` is nan, then the check becomes `abs(np.nan) in [np.nan]` which is `False`, so the function returns `True` when it shouldn't.https://git.ligo.org/lscsoft/bilby/-/issues/557Default "walks" for dynesty2021-05-11T06:29:10ZMatthew PitkinDefault "walks" for dynestyIn the dynesty sampler [doc string](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/dynesty.py#L111) it states that the default is given by "defaults to `ndim * 10`". However, in the default kwargs (and when running t...In the dynesty sampler [doc string](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/dynesty.py#L111) it states that the default is given by "defaults to `ndim * 10`". However, in the default kwargs (and when running the code) it seems to default to 100 even though [this line](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/dynesty.py#L222) is present. Could either the docstring, or the way the default it set, be fixed?https://git.ligo.org/lscsoft/bilby/-/issues/546Matplotlib error at dynesty checkpoint plot2022-12-12T04:24:08ZColm Talbotcolm.talbot@ligo.orgMatplotlib error at dynesty checkpoint plotBug identified by @philip.relton.
I think the solution is to either:
- add this error into the caught exceptions, e.g., [here](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/dynesty.py#L615).
- catch a generic `Exce...Bug identified by @philip.relton.
I think the solution is to either:
- add this error into the caught exceptions, e.g., [here](https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/sampler/dynesty.py#L615).
- catch a generic `Exception` to avoid any future issues. This is generally not optimal, but may be safe enough here.
```python
/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/ticker.py:2072: RuntimeWarning: overflow encountered in multiply
ticks = np.arange(low, high + 1) * step + best_vmin
Traceback (most recent call last):
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/bilby_pipe/data_analysis.py", line 267, in main
analysis.run_sampler()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/bilby_pipe/data_analysis.py", line 242, in run_sampler
**self.sampler_kwargs,
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/bilby/core/sampler/__init__.py", line 182, in run_sampler
result = sampler.run_sampler()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/bilby/core/sampler/dynesty.py", line 365, in run_sampler
out = self._run_external_sampler_with_checkpointing()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/bilby/core/sampler/dynesty.py", line 450, in _run_external_sampler_with_checkpointing
self.plot_current_state()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/bilby/core/sampler/dynesty.py", line 624, in plot_current_state
fig.tight_layout()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/figure.py", line 2476, in tight_layout
pad=pad, h_pad=h_pad, w_pad=w_pad, rect=rect)
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/tight_layout.py", line 362, in get_tight_layout_figure
pad=pad, h_pad=h_pad, w_pad=w_pad)
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/tight_layout.py", line 111, in auto_adjust_subplotpars
tight_bbox_raw = union([ax.get_tightbbox(renderer) for ax in subplots
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/tight_layout.py", line 112, in <listcomp>
if ax.get_visible()])
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/axes/_base.py", line 4361, in get_tightbbox
bb_yaxis = self.yaxis.get_tightbbox(renderer)
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/axis.py", line 1162, in get_tightbbox
ticks_to_draw = self._update_ticks()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/axis.py", line 1080, in _update_ticks
major_labels = self.major.formatter.format_ticks(major_locs)
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/ticker.py", line 259, in format_ticks
self.set_locs(values)
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/ticker.py", line 689, in set_locs
self._set_format()
File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py37-20201027/lib/python3.7/site-packages/matplotlib/ticker.py", line 785, in _set_format
loc_range_oom = int(math.floor(math.log10(loc_range)))
OverflowError: cannot convert float infinity to integer
```https://git.ligo.org/lscsoft/bilby/-/issues/544dynesty "extremely inefficient" warnings only appear at end of log file2022-09-29T09:51:53ZDavid Keiteldavid.keitel@ligo.orgdynesty "extremely inefficient" warnings only appear at end of log fileI did some test runs with `rwalk_dynesty` setting and it turned out the first were badly tuned; `dynesty` actually through warnings about the walks being `extremely inefficient` but those only got printed in bulk at the very end of the l...I did some test runs with `rwalk_dynesty` setting and it turned out the first were badly tuned; `dynesty` actually through warnings about the walks being `extremely inefficient` but those only got printed in bulk at the very end of the log file, after the run finished, not when they occurred. So I suspect there's a missing flush of some sort somewhere.
My use case is parallel_bilby, but I'm opening the issue here since I assume it's calling dynesty by way of the main bilby package, or at least the same workaround would go into both...?
Reported before to @gregory.ashton who thinks this could be fixed on bilby's side.https://git.ligo.org/lscsoft/bilby/-/issues/543fix linebreaks in dynesty logging2021-08-24T10:19:22ZDavid Keiteldavid.keitel@ligo.orgfix linebreaks in dynesty loggingUsing dynesty through `parallel_bilby`, the logfile entries of the `iter: 19 | bound: 0 | nc: 1 | ncall:` type don't seem to have proper unix line-breaks, showing up as `^M` in several tools including `less` and `emacs`, which is at leas...Using dynesty through `parallel_bilby`, the logfile entries of the `iter: 19 | bound: 0 | nc: 1 | ncall:` type don't seem to have proper unix line-breaks, showing up as `^M` in several tools including `less` and `emacs`, which is at least a mild annoyance when parsing them by eye or scripts.
I'm opening the issue here since @gregory.ashton mentioned this should be on the side of the main bilby package, not parallel.https://git.ligo.org/lscsoft/bilby/-/issues/537Bugs in reweighing samples2022-09-09T09:01:03ZKa-Lok LoBugs in reweighing samplesHi,
While trying to reweigh samples using the routine `bilby.core.result.reweight`, I encountered two bugs (in `bilby-v1.0.2`)
1. In `bilby.core.result.get_weights_for_reweighting`, it assumed that the label for each row in `result.pos...Hi,
While trying to reweigh samples using the routine `bilby.core.result.reweight`, I encountered two bugs (in `bilby-v1.0.2`)
1. In `bilby.core.result.get_weights_for_reweighting`, it assumed that the label for each row in `result.posterior` ranges from [0, len(result.posterior)-1] which is _not necessarily the case_ especially rejection sampling was used prior to calling this `get_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](https://git.ligo.org/lscsoft/bilby/-/blob/1.0.2/bilby/core/result.py#L123) 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](https://git.ligo.org/lscsoft/bilby/-/blob/1.0.2/bilby/core/result.py#L174)
from
```
return posterior[keep]
```
to
```
return posterior[keep].reset_index(drop=True)
```
2. It seems that the action of doing `convert_to_flat_in_component_mass_prior` does not commute with doing `bilby.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 since `convert_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](https://git.ligo.org/lscsoft/bilby/-/blob/1.0.2/bilby/gw/prior.py#L67) 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.https://git.ligo.org/lscsoft/bilby/-/issues/533Follow-up from "Adding dnest4 Sampler"2020-10-16T03:57:32ZMoritz HuebnerFollow-up from "Adding dnest4 Sampler"The following discussions from !849 should be addressed:
- [ ] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199274):
> Why are you using estimates of the prior widths and centres...The following discussions from !849 should be addressed:
- [ ] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199274):
> Why are you using estimates of the prior widths and centres? You can get these using the `.minimum` and `.maximum` attributes of the prior classes.
- [x] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199275):
> This line seems superfluous to me, why not just pass `get_random_draw_from_prior` to the `_DNest4Model`?
- [x] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199276):
> These inline comments seem unnecessary
- [x] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199277):
> These print statements should be removed.
- [x] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199278):
> All this commented code should be removed.
- [x] @colm.talbot started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199279):
> It looks like this is duplicating a bunch of code from the `pymultinest` implementation. We should aim to reduce this duplication.
- [x] @moritz.huebner started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199518):
> Aside from `FakeSampler`, the other samplers are listed in alphabetical order. I know this is minor, but it would be helpful readability,
- [x] @moritz.huebner started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199521):
> We don't support python 2 anymore so please don't add `__future__` imports
- [ ] @moritz.huebner started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199522): (+2 comments)
> We usually don't use simple `assert` statements, because they are hard to debug for users. Could you add a helpful exception.
>
> For example
> ```
> if b <= a:
> raise ValueError("Some helpful information")
> ```
- [x] @moritz.huebner started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199525):
> Agree with Colm here.
> ```
> for key in self.search_parameter_keys:
> width = self.priors[key].maximum - self.priors[key].minimum
> center = (self.priors[key].maximum + self.priors[key].minimum) / 2.0
> widths.append(width)
> centers.append(center)
>
> ```
- [x] @moritz.huebner started a [discussion](https://git.ligo.org/lscsoft/bilby/-/merge_requests/849#note_199526):
> The return is unnecessary here1.0.3