search

stan-dev / example-models

Example models for Stan
http://mc-stan.org/
766 stars 478 forks source link

planetary motion case study. #189

Closed charlesm93 closed 3 years ago

charlesm93 commented 3 years ago

Full title, "Bayesian Model of Planetary Motion: exploring ideas for a modeling workflow when dealing with ordinary differential equations and multimodality". I think the title says it all.

Requesting @bbbales2 @jgabry as potential reviewers. Also tagging @andrewgelman, as he's listed as a co-author.

jgabry commented 3 years ago

Hey @charlesm93, at first glance this looks awesome! I haven't had a chance to thoroughly review it yet because I'm a bit swamped at the moment, so it might take me a little while to get to it to all of it. But don't hesitate to remind me about it, I definitely think this is a great case study to have.

charlesm93 commented 3 years ago

I used cmdstanr to fit the model and rstan to do the diagnostics. I agree it'd be good to only use one or the other, and my vote goes to cmdstanr. The problem is I saved the fits as rstan objects, as opposed to using cmdstanr to save the csv files. So I'll need to rerun the models.

@jgabry Is there is way of extracting the csv files from the rstan object?

jgabry commented 3 years ago

@jgabry Is there is way of extracting the csv files from the rstan object?

Hmm, I don't think so unfortunately. RStan can read in from CSV but it doesn't automatically save those CSVs anywhere and doesn't know how to rewrite them. So I think you'd need to refit with CmdStanR to get them.

charlesm93 commented 3 years ago

Thanks @bbbales2 for taking a stab at this. Really appreciate. I've removed the rstan dependency, and I'll push the updated code. There are one or two plotting questions I want to address, I'll post about them on discourse.

charlesm93 commented 3 years ago

Thanks to @jgabry (https://discourse.mc-stan.org/t/tweaking-trace-plots-with-bayesplot/19058), I finished the figures in the paper. I think I've address all of @bbbales2's comments. Waiting for a final approval to merge this.

bbbales2 commented 3 years ago

@charlesm93 cool thanks, I put it on my list. Might be a day or two

charlesm93 commented 3 years ago

Hi,

  1. (and 2) The pdf file is more portable. It's easier to share, without requiring auxiliary files needed by an html file. That said I agree with the consistency argument. I switched the markdown file so that it renders an html bookdown and removed the html file.

  2. The script to generate the RDS file is in the notebook -- and in the rendered PDF, I just double-checked -- but it is not being run because the fits which get saved are not created. That's because some of the fits take 1 - 2 hours to run. It doesn't make much sense to wait hours for the notebook to render every time I make an edit. And that way, the reader has the option to recreate the RDS files or used the saved ones.

  3. That's my bad, I missed the "show more" button from GitHub. Which is funny, because I remember thinking, oh, maybe Ben only read the intro and the conclusion. But that's also why I use the thumbs up system. I'll address the other comments.

Internet says software is plural: https://www.dictionary.com/e/is-software-a-mass-noun/ Also, it's already notable. Indeed, it is noted. That is why it's in a footnote :P.

The plural is intended. Andrew and I have each contributed to several softwares. In my case, Stan, Torsten and mrgsolve. Hence the "notably" in the footnote to indicate that Stan stands out amongst the softwares we've been working on.

No need for bespoke language to talk about bespoke things.

"bespoke" is a perfectly sensible word to use in this context. Since you're the native speaker, I ran this by a peer writing fellow (at Columbia's writing center) and to them, it's totally fine to use the word here.

The latter can be done in several ways and an agile use of plots proves extremely helpful.

Ok, this deserves to be clarified. I'm switching to this: "There are several ways to visualize simulated data and an agile use of plots proves extremely helpful."

This should run with default imports and whatnot. Just like if someone comes into the folder and presses "knit" it should knit. But I'll figure it out once the RDS's are there.

The RDS files have been added. Also .libPaths("~/Rlib/") won't cause any issue, since R will still search the default lib path. It's just compatible with my setup. What can create an issue isset_cmdstan_path()`. If readers really work through the code, they can always comment out this line.

EDIT: All your comments should now have a response or a thumbs up. Thanks for reading this carefully.

bbbales2 commented 3 years ago

I made a couple edits. Check my commit. There are only a few things dangling now.

.libPaths("~/Rlib/")

This caused a problem with my computer. I couldn't load cmdstanr. I was able to open the project up and just click knit and it ran fine with what I have committed. I added a README.md for how to regenerate the data.

charlesm93 commented 3 years ago

Thanks @bbbales2. Your changes look good. I edited the README a bit + some minor edits you suggested.

bbbales2 commented 3 years ago

@charlesm93 What you did looks good. There were a couple comments still hidden (I had to click the hidden comments thing twice to get them). I just made changes that would make me happy, so have a look and if you okay those we're good to go.

charlesm93 commented 3 years ago

@bbbales2 your edits look good to me. I'll merge the PR.