Skip to content

Fix passing of proposals and bounds in the dynesty 3 api - #1117

Open
JasperMartins wants to merge 5 commits into
bilby-dev:mainfrom
JasperMartins:remove-old-dynesty-fixes
Open

Fix passing of proposals and bounds in the dynesty 3 api#1117
JasperMartins wants to merge 5 commits into
bilby-dev:mainfrom
JasperMartins:remove-old-dynesty-fixes

Conversation

@JasperMartins

Copy link
Copy Markdown
Contributor

This MR fixes the issues discussed in #1065.

Closes #1065

Moved from ColmTalbot#4

@mj-will mj-will added this to the 3.0.0 milestone Jul 27, 2026
@JasperMartins

Copy link
Copy Markdown
Contributor Author

I'm unsure about the failing jax-backend test. I don't think it is related to this MR 🤔

@ColmTalbot ColmTalbot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Once we get the CI fixed, I think we can get this merged in.

@ColmTalbot
ColmTalbot force-pushed the remove-old-dynesty-fixes branch from 5fc3625 to 458d749 Compare August 11, 2026 18:10
@ColmTalbot
ColmTalbot enabled auto-merge August 17, 2026 11:44

@ColmTalbot ColmTalbot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, actually, this has broken the default behaviour when not using the live-point sampling.
The docs job timing out is because the default bound for the uniform sampling is now no bound, which is horribly inefficient.

f"estimated chain length. An average of {2 * internal_sampler.nact} "
f"steps will be accepted up to chain length {internal_sampler.maxmcmc}."
)
elif kwargs["bound"] == "live":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to reinstate this clause.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, bound = "none" could be dropped entirely as a default keyword argument for the dynesty sampler. That would make "live-multi" (or whatever the upstream default bounding option is if it ever changes) the default option. If it is preferred that the behavior of "live" stays the default, I'll reinstate the clause, otherwise I'll push a fix that removes the default option entirely.

@mj-will mj-will added sampling Issues about sampling algorithms, their efficiency and robustness dynesty bug Something isn't working labels Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dynesty sampling Issues about sampling algorithms, their efficiency and robustness

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: The dynesty 3 api passes neither the bound-kwarg nor the proposals-kwarg

4 participants