Skip to content
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

use constant gap for rank plots #2180

Merged
merged 5 commits into from
Dec 8, 2022
Merged

use constant gap for rank plots #2180

merged 5 commits into from
Dec 8, 2022

Conversation

aloctavodia
Copy link
Contributor

@aloctavodia aloctavodia commented Dec 5, 2022

Description

Fix #2177. That issue is a good example of where we will benefit from decoupling computation from plotting. Currently, we compute the ranks, use the values to set the space between chains and plot that. And repeat for the next dimension of the variable.

In the meantime, this PR takes a simpler approach and set a constant gap, of two times the expected height of the uniform distribution. When the rank plot is fine this gap should be enough to make the plots looks nice. Although it will allow overlaps between bars from different chains when the rank deviates more than 2 times the expected uniform distribution. I think that's fine, because if you have such overlap then, you have bigger issues than aesthetics :-)

Checklist

  • Follows official PR format
  • Includes a sample plot to visually illustrate the changes (only for plot-related functions)
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

output


📚 Documentation preview 📚: https://arviz--2180.org.readthedocs.build/en/2180/

@codecov
Copy link

codecov bot commented Dec 5, 2022

Codecov Report

Merging #2180 (8871cb5) into main (7091b15) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2180   +/-   ##
=======================================
  Coverage   89.95%   89.96%           
=======================================
  Files         119      119           
  Lines       12398    12401    +3     
=======================================
+ Hits        11153    11156    +3     
  Misses       1245     1245           
Impacted Files Coverage Δ
arviz/plots/backends/matplotlib/rankplot.py 96.92% <100.00%> (ø)
arviz/plots/backends/matplotlib/traceplot.py 98.86% <100.00%> (+0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

sounds good, thanks!

Left a comment on the default handling line

arviz/plots/backends/matplotlib/traceplot.py Outdated Show resolved Hide resolved
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
CHANGELOG.md Outdated
@@ -23,6 +23,8 @@

### Maintenance and fixes
- Fix dimension ordering for `plot_trace` with divergences ([2151](https://github.com/arviz-devs/arviz/pull/2151))
- Fix gap for `plot_trace` with option `kind="rank_bars"` ([2180](https://github.com/arviz-devs/arviz/pull/2180))
Copy link
Member

Choose a reason for hiding this comment

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

this ended up in the 0.14 section instead of the unreleased, other than that it is good to merge

@aloctavodia aloctavodia merged commit 3c82bdd into main Dec 8, 2022
@aloctavodia aloctavodia deleted the rank_gap branch December 8, 2022 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plot trace for rank bars need the bottoms to be aligned for hierarchical model
2 participants