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

Violin plot #316

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

PaulinaMartin96
Copy link
Contributor

@PaulinaMartin96 PaulinaMartin96 commented Jul 6, 2021

A recipe for violin plots was added. Following ArviZ's convetion for violin plots, kwarg combined was added. If true, chains are appended and only one plot per parameter is returned. In this case, colordim := :chain. Otherwise, a violin plot is returned for every chain and every parameter. colordim can be set to :chain or :parameter as shown below. Discrete parameters are plotted as defined in StatsPlots.jl.

using MCMCChains
using StatsPlots

n_iter = 100
n_name = 3
n_chain = 2

val = randn(n_iter, n_name, n_chain) .+ [1, 2, 3]'
val = hcat(val, rand(1:2, n_iter, 1, n_chain))

chn = Chains(val, [:A, :B, :C, :D])
violinplot(chn, size = (200, 1000)) #violinplot(chn, combined = true, colordim = :chain)

image

violinplot(chn, combined =  false, colordim = :chain)

image

violinplot(chn, combined = false, colordim = :parameter, size=(800, 800))

image

@cpfiffer
Copy link
Member

cpfiffer commented Jul 6, 2021

I think you probably ended up making a new branch from your other PR's branch in #307, rather than from the master branch. Each PR should be independent from one another unless they are explicitly planned to be merged in sequence or something, which we very rarely do. It might be worth investigating some time on figuring out how to make it so your two commits 4fc9d62 and d1cadce are added to master rather than on top of the commits in #307.

  • Make sure to increment the version number for this one -- I think if we end up merging Make Chains objects display only information and not statistical eval #307 first, this PR should be associated with 4.15.0.
  • Please add a demo plot and a minimum working example so the reviewer knows what to expect. It's also generally considered a good idea to do a brief writeup in general, rather than just use the commit title you currently have.

@PaulinaMartin96 PaulinaMartin96 changed the title Pm/violin plot Violin plot Jul 13, 2021
@PaulinaMartin96 PaulinaMartin96 marked this pull request as ready for review July 13, 2021 03:21
@cpfiffer
Copy link
Member

FYI @PaulinaMartin96 the tests are failing on this one. You can see why the test failures are happening by clicking "Details" in the testing box towards the bottom of the thread on GH:
image

@delete-merged-branch delete-merged-branch bot deleted the branch TuringLang:master December 24, 2021 10:30
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.

2 participants