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

Remove extra space in mcmc_areas #230

Merged
merged 6 commits into from
Aug 7, 2020
Merged

Conversation

jgabry
Copy link
Member

@jgabry jgabry commented Jun 22, 2020

Fixes #218

Copying from my comment in #218:

after playing around with a bunch of options and trying with different numbers of parameters, what ended up working best (I think) was to use an additive expansion that decreases with the number of parameters. I put this on a branch remove-extra-space-mcmc_areas. Here's a comparison with the current master branch:

For 2, 4, 10, and 20 parameters (simulated from standard normals) here is what the current master branch produces:
areas_old

and here is what the version in this PR looks like for the same parameters:

areas_new

@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2020

Codecov Report

Merging #230 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #230   +/-   ##
=======================================
  Coverage   99.28%   99.28%           
=======================================
  Files          31       31           
  Lines        3928     3939   +11     
=======================================
+ Hits         3900     3911   +11     
  Misses         28       28           
Impacted Files Coverage Δ
R/mcmc-intervals.R 99.22% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4ed652...ae68e50. Read the comment docs.

@jgabry jgabry requested a review from tjmahr August 6, 2020 23:34
@tjmahr
Copy link
Collaborator

tjmahr commented Aug 7, 2020

While we are under the hood for this one, I'd like to fix the clipping where the line in the top panel gets cut off.

image

The demos you supply here appear to fix this problem, but I can still get it if I use area_method = "equal areas" in this version.

I'm trying the geom_blank() strategies of adjusting plotting area by plotting invisible data.

Copy link
Collaborator

@tjmahr tjmahr left a comment

Choose a reason for hiding this comment

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

Can you try out your diagnostic plots using the updated version?

@jgabry
Copy link
Member Author

jgabry commented Aug 7, 2020

While we are under the hood for this one, I'd like to fix the clipping where the line in the top panel gets cut off.

Good call, thanks.

Can you try out your diagnostic plots using the updated version?

Yup, will check now.

@jgabry
Copy link
Member Author

jgabry commented Aug 7, 2020

Can you try out your diagnostic plots using the updated version?

Do you mean the same set of plots I made above? I made them again (although I forgot to set a seed last time so they'll look a bit different). They still look good after your change. Here's using default arguments:

areas_new-default

And here's using prob = 2/3, prob_outer = 0.9, point_est = "mean":

areas_new-default-2

@tjmahr tjmahr merged commit 52f1a9f into master Aug 7, 2020
@tjmahr tjmahr deleted the remove-extra-space-mcmc_areas branch August 7, 2020 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove extra space from mcmc_areas
3 participants