Allow deprecated syntax for specified versions of specified packages

[andrjohns]

The main blocker for the rstan -> CRAN transition is the number of packages that still have not updated their deprecated syntax, despite plenty of notice.

To workaround this, the current PR adds an 'exception list', where specified packages at specified versions (currently only the broken reverse-dependencies) have their Stan code canonicalised/updated at installation-time for compatibility with stan 2.36+.

If a version for one of these packages changes, then the automatic canonicalisation does not occur and the author would be required to update their Stan syntax for their package to build.

This allows for a compromise where abandoned/unmaintained packages are no longer blocking rstan updates, while any packages still receiving active development will be forced to update their syntax once a new version of rstan is published.

As part of testing this, I've created an rstan branch at the 2.39 release point here: rstan@v2.39.0 and added handling for the RNG changes (ecuyer->mixmax).

With these rstantools changes and that rstan branch, I'm able to build all CRAN reverse-dependencies under both combinations (2.32 rstan + 2.39 StanHeaders) and (2.39 rstan + 2.39 StanHeaders) except for two (tmbstan and rmdcev)

[andrjohns]

Pinging @bgoodri for the rstan release perspective, and @WardBrian on the canonicalisation approach (thanks!)

[WardBrian]

The overall approach seems prudent and sound.

Two questions:

• Do any of the affected packages use #includes? Are those already processed by the time the canonicalization would happen?
• Why 2.32? I believe 2.33 would still work, and had a few other improvements that might benefit pretty printing (though it's been a while, maybe they're the same in that regard)

[andrjohns]

Do any of the affected packages use #includes? Are those already processed by the time the canonicalization would happen?

Yeah, it uses the rstan::stanc_process function to first replace all #include chunks with the respective code so the canonicalisation is applied to the whole model

Why 2.32? I believe 2.33 would still work, and had a few other improvements that might benefit pretty printing (though it's been a while, maybe they're the same in that regard)

Pretty much just because I just copied the stanc.js file from the current CRAN rstan and it worked, so I didn't look anywhere else 😅

[WardBrian]

Sounds good to me

[andrjohns]

Reverse-dependencies check with this rstantools branch plus CRAN rstan and StanHeaders 2.39.0 is here: https://github.com/andrjohns/rstan/actions/runs/29497863526#summary-87637709672

2 install failures: tmbstan (PR opened) and rxode2ll), and ~20 test/vignette/example errors - mostly issues with optimizing and unconstraining parameters

[jgabry]

Thanks @andrjohns, looks good to me. @bgoodri?

[andrjohns]

The majority of the revdep test failures were caused by Stan's optimize now returning an extra converged__ col that rstan wasn't expecting.

Patching that out of the StanHeaders includes brings the revdep failures down to only 5: https://github.com/andrjohns/rstan/actions/runs/29560600034#summary-87834072879

EDIT: Down to three failures: https://github.com/andrjohns/rstan/actions/runs/29574108492#summary-87877836657

[andrjohns]

There are now only four packages showing install/test failures with CRAN rstan and StanHeaders 2.39.0:

• tmbstan: PR Merged
• rxode2ll PR Merged
• miniLNM: Existing CRAN failures - Unrelated to StanHeaders
• rmdcev: Initialisation issues with cholesky factors in tests

The only one I haven't quite figured out yet is the rmdcev failures, but we're pretty much on the home stretch for StanHeaders 2.39 imo

[jgabry]

Awesome, thanks @andrjohns!

[andrjohns]

Fixed up the rmdcev issues, there is now only one install failure and no test/vignette/example failures for CRAN rstan & StanHeaders 2.39.0: https://github.com/andrjohns/rstan/actions/runs/29679765224#summary-88182719469

• The PR for the tmbstan failure has already been merged
• The miniLNM failure is unrelated to StanHeaders

For rstan 2.39 + StanHeaders 2.39.0, there's an additional installation failure and more test/example/vignette failures: https://github.com/andrjohns/rstan/actions/runs/29679765224#summary-88180287611

• PR for the rmdcev failure has been opened
• I'll keep working through the remaining example/test/vignette issues

@bgoodri @jgabry how did you want to handle the submission process? Should we submit rstantools and StanHeaders 2.39 now/soon (my vote) or wait until the rstan issues are also resolved?

[andrjohns]

All reverse-dependencies for StanHeaders are now green (the tmbstan fixes have already been merged).

@jgabry happy for this to go CRAN? Then all is clear for @bgoodri to submit StanHeaders

[jgabry]

Awesome, yeah I'm happy for this go to CRAN! Let me know when you submit and I'll keep an eye out for the CRAN email so I can confirm the maintainer change.

[jgabry]

Let's just merge #158 before releasing (I'll do it after checks pass). Also, since you'll be doing the release, check out my comment there about the new way the pkgdown website works (much simpler now)

Edit: #158 is merged now

[jgabry]

Looked at this again and noticed a few things, sorry I missed them before! Hopefully they're quick fixes. Other than these things, I think it's CRAN ready.

[jgabry]

These are going to run every time rstan_config() runs, right? For example, for survstan:

first run: offset -> offset_par
second run: offset_par -> offset_par_par

[jgabry]

For the CRAN revdep workaround this probably isn't an issue, but I think this would be a problem when any package developer locally runs rstan_config more than once.

We actually have a test for this, but it doesn't catch this because it's using a fake RStanTest package, not one of the packages in stanc_exceptions: (

rstantools/tests/testthat/test-rstan_config.R

Lines 83 to 96 in e03b02a

	#--- 3. rerun config, check that file.mtimes are identical --------------------
	test_that("Unmodified Stan src files are not overwritten", {
	if (!run_all_tests) {
	skip_on_cran()
	skip_on_travis()
	}
	pre_mtime <- lapply(stan_files, check_mtime, pkg_dest_path = pkg_dest_path)
	pre_mtime <- do.call(c, pre_mtime)
	rstan_config(pkg_dest_path)
	post_mtime <- lapply(stan_files, check_mtime, pkg_dest_path = pkg_dest_path)
	post_mtime <- do.call(c, post_mtime)
	expect_identical(pre_mtime, post_mtime)
	})

)

[jgabry]

I think is_excepted is actually ignored, so e.g. every future survstan will get the offset_par -> offset change instead of forcing them to change in the future, which I think was the original intent

[jgabry]

I see your previous commit that removed using is_excepted because of disbayes mangling, which makes sense, but I think we need to keep it for the other packages.

[jgabry]

Do we need to check $errors in addition to $result?