search

wwiecek / baggr

R package for Bayesian meta-analysis models, using Stan
GNU General Public License v3.0
46 stars 12 forks source link

Fix stan typos, cleanup files #157

Closed andrjohns closed 1 year ago

andrjohns commented 1 year ago

This PR updates some minor typos in your Stan code (using the _lpmf suffix in a sampling statement), and removes files that are automatically provided by rstantools

wwiecek commented 1 year ago

Thanks so much @andrjohns ! You read my mind: last week I fixed these typos in a local branch, but haven't committed them because I wasn't sure what to do about Makevars. So it's great to know that the makevars files should no longer be included.

However, I get a file too big compilation error on Windows (on Ubuntu I think it's alright) with R 4.2.2, rstan and StanHeaders 2.26.13.

When digging into this last week, I saw that someone on Stan Discourse had a similar problem. There was also a post by Frank Harrel on this. Is there some guidance on what the Makevars should be for developers using rstantools? (Especially Makevars.win) I assume I should have something in ~/.R/? Maybe I missed some other necessary step in updating the package to new R/rstan versions?

Any suggestions would be great. I've been trying to figure this out on my own but it's a bit confusing because the instructions concerning Makevars have changed over years, so sometimes Google just turns up some suggestions that no longer apply.

andrjohns commented 1 year ago

Sorry for missing this! I've updated the PR with the fix for this error on Windows

andrjohns commented 1 year ago

Would you be able to merge this and release to CRAN when you can? Thanks!

wwiecek commented 1 year ago

Hi @andrjohns it seems like this configure.win fix nearly does what's needed, but there is one last stumble.

If I run pkgbuild::compile_dll() on my Windows machine and look at Makevars, I can see PKG_CXXFLAGS += -Wa,-mbig-obj added to Makevars.win. But a few seconds later, presumably when config file runs rstantools::rstan_config() in the last line of your fix, the new flags disappear. I use rstantools 2.2.0. Am I doing this wrong?

andrjohns commented 1 year ago

Sorry about that! I've updated the configure.win file and verified that it works locally for me under Windows and 2.26, can you check on your system?

wwiecek commented 1 year ago

Thanks, this all works very nicely now. I will merge this but open a separate issue to ask you a couple questions, basically I'd be really grateful if you could give me some more info on this problem.

wwiecek commented 1 year ago

PS: FYI, this will go to CRAN in a few days, there is another critical bug that CRAN flagged recently so I need to look into that.