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.
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.
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.
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.
|
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 :