stan-dev
Upcoming Stan release removes old array syntax, new RStan needed
See https://discourse.mc-stan.org/t/cmdstan-stan-2-33-release-candidate/32531. This one actually removes the old array syntax, not just a deprecation warning.
@bgoodri @andrjohns @hsbadr Is RStan with new array syntax ready to be submitted to CRAN?
If it's ready, I would go ahead and submit it ASAP, I think CRAN is back from vacation
@bgoodri if you're short on time or overloaded with other work, did you want to temporarily assign someone else as the maintainer for CRAN? Then we can get everything up to date ASAP and take some of the pressure off.
@hsbadr would be my first suggestion, but I'm also happy to take it if he doesn't have capacity either
I can do it. I have just been swamped this past month with a conference and teaching. We need to merge @andrjohns PR about switching the javascript library, but then 2.26.x is pretty much ready I think. Hopefully, we can go to 2.32.x almost immediately afterward because the error messages in 2.26 are not great and it doesn't have the tool to convert to the new array syntax. I imagine it will take a bit before all the packages actually convert to the new array syntax though.
Thank you for all your efforts in getting this to work!
From the perspective of brms, not having the new array syntax in rstan is a huge problem since the latest cmdstan(r) will not support the old one anymore. So without the new rstan, I can either give up on one of the backends (which I won't), remove brms from CRAN (which I won't either), or add a lot of code that outputs new or old array syntax depending on the stan version that is used. The latter I would like to avoid as well, of course, since it complicates the whole code structure and would be a lot of additional work just for a temporary work around.
So from my side, I really cannot wait to have rstan 2.26+ on CRAN!
@bgoodri Here are all the changes needed for rstan 2.26 on CRAN, I've run the reverse dependency checks and also checked on winbuilder and all passes.
StanHeaders needs one header updated for c++17: https://github.com/stan-dev/math/compare/StanHeaders_2.26...rstan226-cran
Rstan changes (including the change from V8): https://github.com/stan-dev/rstan/compare/StanHeaders_2.26...rstan-cran
Can you please either submit ASAP or let me know what else you need. The longer it takes to get rstan up, the worse it's going to be when dealing with the array syntax in packages
I've run the reverse dependency checks and also checked on winbuilder and all passes.
Thank you!
Can you please either submit ASAP or let me know what else you need. The longer it takes to get rstan up, the worse it's going to be when dealing with the array syntax in packages
Agree, let me know if there's anything I can do to help.
This one actually removes the old array syntax, not just a deprecation warning.
This is a big change that requires a lot of effort. The experimental tested the latest development branches of Math and Stan. Everything is ok except this change (see https://github.com/stan-dev/rstan/actions/runs/6012867756). We'll need to update all Stan code in RStan tests and ask all dependencies to adopt the new array syntax.
Rstan changes (including the change from V8): StanHeaders_2.26...rstan-cran
Do we need to rebase this for the experimental branch?
@hsbadr would be my first suggestion, but I'm also happy to take it if he doesn't have capacity either
Does CRAN allow for multiple maintainers? If so, I'd add more maintainers with @bgoodri to keep RStan alive.
This is a big change that requires a lot of effort. The
experimentaltested the latest development branches of Math and Stan. Everything is ok except this change (see https://github.com/stan-dev/rstan/actions/runs/6012867756). We'll need to update all Stan code in RStan tests and ask all dependencies to adopt the new array syntax.
Regarding the dependencies, if we give them a month or two of notice it should be OK for us to submit the update. So we could do 2.26 now and then 2.32 this fall. Especially if we give them a way to update their code (there's a tool to convert to the new syntax right?) then we satisfy CRAN's policy and we shouldn't keep holding up Stan development (plus having 2.26 out is going to lead to a lot of confusion if the error messages are messed up). I'm happy to help out with any of this (dependencies, test code), just let me know if/how I can be useful.
Do we need to rebase this for the experimental branch?
I'm not sure. @andrjohns @bgoodri?
Does CRAN allow for multiple maintainers? If so, I'd add more maintainers with @bgoodri to keep RStan alive.
No, unfortunately. They want a single point of contact.
Thanks again for all the work you've done to help with this already!
We are go for StanHeaders with the additional size -> math::size thing. I am a bit perplexed as to how StanHeaders 2.26.27 ultimately worked with survstan and all the other packages with rstan 2.21.x but not with rstan 2.26 but I can live without resolving that mystery.
The develop branch of rstan would satisfy the reverse dependency checks if not for that one error that is now fixed with StanHeaders. It is similar to @andrjohns branch, especially in the sense that it uses his QuickJSR package rather than V8 to handle the javascript stanc, which allows us to drop the old C++ stanc and parse both the old and the new array syntax. So, I think we should do a rstan 2.26 right after the StanHeaders 2.26.28 gets through CRAN so that @paul-buerkner just has to push a brms that requires rstan >= 2.26 .
I do want to get both StanHeaders and rstan up to Stan 2.32 right afterward, which as I understand it is basically the experimental branch. That release accepts the old array syntax but warns that it will be unsupported in 2.33. So, we will presumably have to prod a bunch of packages to run the canonicalizing tool on their Stan files.
Uploading the new StanHeaders (that only changed one size to math::size) somehow broke the following rstantools-using packages on Fedora and M1:
Rglt
bmlm
eggCounts
It is the same error that we have encountered and fixed before, so I don't understand how it got triggered in these few circumstances by a StanHeaders that should not have affected their C++ code.
I have no idea unfortunately. That said, it is totally okay for rstan to break some downstream packages with a major release. We desperately need rstan 2.26+ on CRAN asap (not just for brms but for many other reasons too). So if figuring out this error would caused another non-trivial delay to the release, I would strongly argue in favor of just allowing these packages to break and be fixed later (or removed from CRAN if no fixe it provided).
It is possible that we can go ahead with the rstan 2.26 because doing so would not make those three packages any "worse" as far as the CRAN checks are concerned than what they already are now that StanHeaders 2.26.28 already broke them. But it is still our responsibility to manage backwards incompatible changes on CRAN, even though in this case, it seems to be due to those packages using the older build process rather than letting rstantools do its thing to regenerate the C++ and the Makevars{.win} files when the package is compiled.
@bgoodri those failures are because the packages aren't using rstantools to update their Makevars, and so are missing flags needed for compatibility with c++17 (the AUTO_PTR_ETC one)
They're easy fixes and I'll open PRs now, all safe to go ahead with the rstan submission
@bgoodri PRs submitted:
I think that's plenty to satisfy CRAN, so please go ahead and submit rstan or let me know if you need anything else
OK. Let me see if I can merge your rstan-cran branch into develop without too many merge conflicts.
On Thu, Sep 7, 2023 at 12:43 PM Andrew Johnson @.***> wrote:
@bgoodri https://github.com/bgoodri PRs submitted:
- bmlm https://github.com/mvuorre/bmlm/pull/23
- Rlgt https://github.com/cbergmeir/Rlgt/pull/6
- eggCounts: No public repo, emailed maintainer
I think that's plenty to satisfy CRAN, so please go ahead and submit rstan or let me know if you need anything else
— Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstan/issues/1084#issuecomment-1710475250, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKWYXW6EJGNUAMNTVTDXZH2TTANCNFSM6AAAAAA36XFGEI . You are receiving this because you were mentioned.Message ID: @.***>
I appreciate the work on this. I just updated my rmsb package to work with the new syntax for cmdstanr only to find that it breaks rstan so I've kind of stuck.
Hopefully just a few more hours Frank.
On Thu, Sep 7, 2023, 5:44 PM Frank Harrell @.***> wrote:
I appreciate the work on this. I just updated my rmsb package to work with the new syntax for cmdstanr only to find that it breaks rstan so I've kind of stuck.
— Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstan/issues/1084#issuecomment-1710805876, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKS7542MJNQQSQLNJCDXZI52XANCNFSM6AAAAAA36XFGEI . You are receiving this because you were mentioned.Message ID: @.***>
@paul-buerkner @harrelfe There is now a RStan on CRAN that accepts both array syntaxes. We will have to do a follow-up RStan based on 2.26 before we can do one based on 2.32 but that shouldn't affect you. It does seem possible to make a Stan program that is too complicated to be parsed by QuickJSR, so that might become an issue for some brms models.
Thanks, nice to see this out. Not sure if I should start a new issue, but with the updated rstan / stanheaders, package building (with ctsem) has a substantial pause (30s maybe?) where it 'looks' like nothing is happening.
That is presumably the parser parsing. I had the same thing with rstanarm (and then it failed). So, if it is just a compile time thing (in the case of ctsem and other packages that use rstantools), I am not too worried. The pause will be less noticeable with 2.32.
happens with the rstantools configure script disabled too, and no compilation occurring, fyi.
happens with the rstantools configure script disabled too, and no compilation occurring, fyi.
This is because we're using a less powerful javascript engine (quickjs instead of V8) which is more portable, but less performant.
There were significant optimisations in stanc3 after 2.26, so once we get StanHeaders 2.32 on CRAN then the parsing time should be ~3x faster
I think I will modify the rmsb package to be for cmdstan and create a new package rmsbr whose sole content is rstan stanmodel objects that rmsb can use , primarily for Windows users. I'd like to avoid some future stan version conflicts. @paul-buerkner what do you think of that?
I think I will modify the rmsb package to be for cmdstan and create a new package rmsbr whose sole content is rstan stanmodel objects that rmsb can use , primarily for Windows users. I'd like to avoid some future stan version conflicts. @paul-buerkner what do you think of that?
I don't think that's necessary (imo), we're past the difficult conflicts and updates so things should be fairly smooth from here on
I think you're right @andrjohns but I wonder if I need to hedge my bets because of future changes to cmdstan.
@hsbadr would be my first suggestion, but I'm also happy to take it if he doesn't have capacity either
Speaking of @hsbadr, looks like still listed as a contributor. Should probably be an author going forward! (Noticed this when trying to update the website)
Hi, @harrelfe. @andrjohns and @bgoodri have been working hard to make sure that things will be smoother after the 2.26 update (which just went live on CRAN) that things will be smoother.
To ensure there are no nasty surprises, I would strongly recommend updating your Stan code so that there are no deprecation warnings. We are going to start removing deprecated functionality. Also, we're not guaranteeing that binary models will be compatible version to version, just the Stan programs will still work if you recompile them. Other than that, there shouldn't be issues with backward compatibility going forward.
I wonder if I need to hedge my bets because of future changes to cmdstan.
Sorry---missed the later comment. The backward compatibility issues will be the same for cmdstan as for rstan and cmdstanr---we plan to remove deprecated functionality. Overall, we're recommending that people use cmdstanr when possible because it's not clear we'll keep upgrading RStan in the future given how much of a pain it is to get things on CRAN given lack of version dependency management and restrictive external unit test compatibility.
@WardBrian Can we temporarily get a patched version of stac3 JS v2.33.0 with soft deprecations (warnings instead of errors)?
I think it wouldn’t be too much work to do so, I think we could cleanly revert the PR which made them errors (but the messages would say “2.33”, which might be confusing if it is 2.33)
Can I ask why this would be useful?
