xfim / ggmcmc

Graphical tools for analyzing Markov Chain Monte Carlo simulations from Bayesian inference
111 stars 31 forks source link

Fix: import reshape2 instead of reshape #17

Closed jucor closed 11 years ago

jucor commented 11 years ago

This should solve issue #15 , hope this helps :)

dmenne commented 11 years ago

No, that's no enough. You have to run the test, for example variable_name is variable.name in reshape2

jucor commented 11 years ago

Great: so where are the unit tests that I can run? I can't find any.

dmenne commented 11 years ago

Build the package which includes an example; there are no unit tests, but a full build/check for CRAN includes this.

jucor commented 11 years ago

Thanks. Weird: I had compiled the package, but didn't see the test. I probably had a no-check flag somewhere, or didn't pay attention. Will get back to it.

jucor commented 11 years ago

PS: I've asked on manipulatr mailing list for any info on broken backward compatibility between reshape and reshape2.

dmenne commented 11 years ago

Instead wasting too many grey cells on the check-flags, use RStudio, create a package project, and Build/Check. I also suggest to explicitly check "roxygenize on Build and Reload", because that what one easily forgets.

The only other change is the _/. in melt.data.frame. But there is also the problem with Rhat and 1 chain, and the nasty histogram warnings.

jucor commented 11 years ago

Thanks Dieter. Looks like there's also cast() which has been renamed acast/dcast depending on the use. The warnings do not worry me so much: they are ugly indeed, but apparently can be fixed.

jucor commented 11 years ago

Fixed the _/., the cast(), and a naughty melt() issue, too. Now the check passes smoothly, and doesn't raise any histogram warnings. Could you please give me examples to reproduce:

If you add them as standalone files and pull-request on my fork jucor/ggmcmc , I will turn them into unit tests and work from there. Thanks!

dmenne commented 11 years ago

You should see the histogram-warning when compiling the standard example. It comes from ggs_histogram/geom_histogram. I suggest the following changes, but cannot get it to work since I do not know the syntax (code below fails in geom_histogram). Feel free to check: http://stackoverflow.com/questions/17271968/different-breaks-per-facet-in-ggplot2-histogram

breaks = tapply(D$value,D$Parameter,function(x){ pretty(range(x), n = nclass.FD(x), min.n = 1) }) f <- ggplot(D, aes(x=value) ) + geom_histogram(breaks=breaks) + facet_wrap(~ Parameter, ncol=1, scales="free") return(f)

dmenne commented 11 years ago

When trying to reproduce the 1-chain problem, I encountered another one which I remember had to do with some coda-related problems; I had already solved that once, but don't remember the details. See Issue #18. Sorry, I have to leave, cannot work on this one currently.

jucor commented 11 years ago

Thanks for your help Dieter! I'll fix what I can/need, and contact @xfim to see if he can pull.

jucor commented 11 years ago

The problem with the multiple binwidths appears to be that breaks can only specify common breaks for all variables. There are a few workarounds, but the desagreement of the warning is small enough and the effort to high for me to push further :-) I suggest filling a separate issue.

jucor commented 11 years ago

Bottom line: the port to reshape2 is done here. The Rhat with 1 chain is also a separate problem (that I'll be glad to look into, too), but at least this issue is ripe for merging. Emailing @xfim. Thanks for the input Dieter!