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

Update deprecated syntax for future rstan compatibility #65

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

andrjohns
Copy link
Contributor

Now that rstan 2.26 is available on CRAN we need to update the deprecated syntax in your package's Stan models, otherwise it will fail to install with an upcoming version of RStan.

The following updates have been made:

  • New array syntax
  • lower is a reserved keyword, changed to input_lower
  • upper is a reserved keyword, changed to input_upper
  • _cdf functions using conditional | notation (i.e., _cdf(y |x))
    • The current CRAN rstan has a bug in recognising this in _cdf functions, so I've updated your syntax to work on the log scale with the _lcdf function. This will let your package be compatible with the current and future parser, and is also more numerically stable with extreme values (bonus!)

More information about the deprecated and removed syntax in Stan can be found here:

If you could merge and submit to CRAN soon, it would be greatly appreciated.

Let me know if you have any questions about these changes.

Thanks!

Copy link
Collaborator

@kkmann kkmann left a comment

Choose a reason for hiding this comment

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

Hi @andrjohns,

thank you so much for raising this, I had no clue that these syntax changes were coming. Nice to see that the the notation is a bit more consistent.

I tried to investigate your proposed changes in a bit more detail and ran into some issues with convergence etc. hence I cannot merge this in its current state. I double checked that this is not related to changes in rstan in the meantime but up to 2.26.23 the old version of oncomsm seems to run fine, tests pass and vignettes build. This is most probably related to the changes around lcdf you included, the rest seems trivial. I might be more comfortable making the required changes sequentially to ensure we don't break the model. I could not find the changes you mention in the online documentation yet - do you have another source (can be a gh issue as well) for me to wrap my head around the required changes?

Is there a way of getting deperecation warnings? Which versions from gh rstantools/rstan do I need to test this against?

@kkmann kkmann linked an issue Sep 29, 2023 that may be closed by this pull request
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.

None yet

2 participants