Skip to content

Use melt_mcmc instead of calling reshape2::melt#246

Merged
jgabry merged 2 commits into
stan-dev:masterfrom
hhau:patch-1
Oct 31, 2020
Merged

Use melt_mcmc instead of calling reshape2::melt#246
jgabry merged 2 commits into
stan-dev:masterfrom
hhau:patch-1

Conversation

@hhau
Copy link
Copy Markdown
Contributor

@hhau hhau commented Oct 29, 2020

Closes #245 (tests pass locally for me)

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 29, 2020

Codecov Report

Merging #246 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
- Coverage   98.34%   98.34%   -0.01%     
==========================================
  Files          32       32              
  Lines        4112     4111       -1     
==========================================
- Hits         4044     4043       -1     
  Misses         68       68              
Impacted Files Coverage Δ
R/mcmc-diagnostics.R 97.53% <100.00%> (-0.01%) ⬇️

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 2dfa48f...a93a75c. Read the comment docs.

Comment thread R/mcmc-diagnostics.R
}

data <- reshape2::melt(x, value.name = "Value")
data$Chain <- factor(data$Chain)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason to also get rid of making Chain a factor? To be honest I'm not sure why that was there in the first place (it may never have been necessary, maybe I thought tapply required factors), but curious if you removed that intentionally?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed it intentionally. I didn’t think it was necessary as tapply calls as.factor on INDEX anyway.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok that makes sense, thanks!

Copy link
Copy Markdown
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

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

Good catch, thanks for this! I just put one tiny question below (or above, wherever it ends up).

Copy link
Copy Markdown
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

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

Thanks again for the PR. Merging now.

@jgabry jgabry merged commit 7fabb3e into stan-dev:master Oct 31, 2020
@hhau hhau deleted the patch-1 branch October 31, 2020 18:03
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.

mcmc_acf errors for some input types

3 participants