-
Notifications
You must be signed in to change notification settings - Fork 21
multimodal hdi feature #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Sounds good to me! No initial thoughts on the implementation, tests, or doco updates apart from - when you're happy with the implementation - we should make sure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements multimodal HDI (Highest Density Interval) estimation for posterior parameters, similar to the functionality provided by ArviZ. The feature allows ChainConsumer to identify and visualize multiple disjoint density bands in multimodal distributions.
- Adds HDI as a new summary statistic option
- Implements multimodal interval detection using density thresholding
- Updates visualization to display multiple intervals with appropriate formatting
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/chainconsumer/statistics.py | Adds HDI enum value to SummaryStatistic |
| src/chainconsumer/chain.py | Adds multimodal configuration field and validation logic |
| src/chainconsumer/analysis.py | Implements HDI computation, interval detection, and multimodal bound conversion |
| src/chainconsumer/plotter.py | Updates bar plotting to handle multiple intervals and format multimodal titles |
| tests/test_analysis.py | Adds comprehensive tests for HDI functionality and ArviZ parity |
| tests/test_plotter.py | Adds tests for multimodal visualization |
| tests/test_translators.py | Updates MCMC configuration for sequential chain method |
| docs/examples/plot_7_multimodal_chains.py | Adds example demonstrating multimodal chain usage |
| docs/examples/plot_5_emcee_arviz_numpyro.py | Minor title correction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @Samreay! I think this is good enough to be reviewed now. Sorry for the spam, I have been messing around with the new copilot reviews |
Samreay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one real comment! Oh and also I think usage.md should have the "Statistics" heading updated in text, even if we dont have an updated stats.png to add to the mix.
Samreay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
|
All good on your end to merge, too? Feel free to merge, if so :) |
|
Done! TY for your reviews @Samreay! |
Hi @Samreay
I am drafting a PR to implement multimodal resilient estimator for the posterior parameters.
In the same way as in
arviz, I am trying to add HDI as a summarizer. Below are examples for the same chain marked as unimodal and multimodalI still should tinker a bit around to propose a proper implementation, as most of the current implementation is written by Codex.
The current way to use it is simply to mark a chain as multimodal when building it and to use the HDI stat to summarize each parameter :