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

CRAN M1 issue - parser failed badly; maybe try installing the V8 package #1154

Open
gowerc opened this issue Feb 18, 2025 · 20 comments
Open

Comments

@gowerc
Copy link

gowerc commented Feb 18, 2025

Hi,

We have an issue with rstan specifically on the CRAN M1 Mac mini systems. For transparency I had originally raised this issue on the Stan forum here however since then we have received an escalation email from CRAN:

Please correct before 2025-03-11 to safely retain your package on CRAN.
Packages using rstan::stanc are failing to install in R-devel
using C23 (which is now the default) with current LLVM or Apple clang.
The rstan maintainer was informed 4 weeks ago and has not produced
an update (and the failure message is nonsense: V8 is installed
and that has not been checked).

I couldn't see any corresponding issue thread here so figured I would open this (please close if you are tracking elsewhere). We are currently considering switching to cmdstanr (as advised) and disabling the relevant tests on CRAN but figured we'd post this issue here in case there is any additional action needed on your side to avoid further issues with the CRAN team.

Full error message for context:

 Error in `stanc(file = file, model_code = model_code, model_name = model_name, 
      verbose = verbose, obfuscate_model_name = obfuscate_model_name, 
      allow_undefined = allow_undefined, allow_optimizations = allow_optimizations, 
      standalone_functions = standalone_functions, use_opencl = use_opencl, 
      warn_pedantic = warn_pedantic, warn_uninitialized = warn_uninitialized, 
      isystem = isystem)`: parser failed badly; maybe try installing the V8 package

https://www.stats.ox.ac.uk/pub/bdr/M1mac/rbmi.out

@wds15
Copy link
Contributor

wds15 commented Feb 18, 2025

I have just been contacted by Prof Ripley pointing to this exact same issue which is also triggered by our OncoBayes2 package on CRAN. If I cannot fix the issue the package may go off CRAN by 11th of March.

@bgoodri or @andrjohns ... any suggestions? Something we can do to help debug and resolve the matter?

@bgoodri
Copy link
Contributor

bgoodri commented Feb 18, 2025

I am aware of it. I do not think there is a good reason why Stan packages should have problems when the C23 standard is enforced in addition to C++17, but that seems to affect the way V8 is built, and now the javascript version of stanc is failing to parse Stan programs that it was previously parsing. If there is anyone who can replicate the error and can try building their package without having V8 installed, it should revert to using the QuickJSR package that @andrjohns wrote, which may work better but historically has had trouble with long and / or complicated Stan programs. Failing that if we can identify the simplest Stan program that triggers the problem, that would be a step forward.

@WardBrian
Copy link
Member

The fact that silent=TRUE is set in the try call might make this harder to diagnose

@andrjohns
Copy link
Contributor

Looking at the stan files in the rbmi and OnceBayes2 packages, I'm pretty sure this is caused by the # handling that we were discussing in #1145, rather than an rstan/stanc-wide issue

You can reproduce the behaviour locally using the RHub docker container for C23:

docker run -it ghcr.io/r-hub/containers/c23:latest
Rscript -e "pak::pak('OncoBayes2')"

@bgoodri
Copy link
Contributor

bgoodri commented Feb 18, 2025

Good. Does anyone have a theory as to what C23 has to do with anything?

@andrjohns
Copy link
Contributor

I think it's just a change in the toolchain, since C23 is using clang - and the current # handling is calling the c compiler/pre-processor.

I think that block might be redundant though, since the block above already looks like it's replacing the #include directives with the file contents?

@andrjohns
Copy link
Contributor

And for a simpler reprex with the C23 docker image:

With QuickJSR default:

pak::pak("rstan")
res1 <- rstan::stanc(model_code="data {}\n // Comment")
res2 <- rstan::stanc(model_code="data {}\n // Comment with #")
# TypeError: not a function
#   at aRz (<input>:235:25)
#   at dK7 (<input>:764:9)
#   at ee (<input>:766:14)
#   at dIb (<input>:25502:3)
# 
# Error in model_cppcode$errors : $ operator is invalid for atomic vectors

And with V8:

pak::pak("V8")
res1 <- rstan::stanc(model_code="data {}\n // Comment")
res2 <- rstan::stanc(model_code="data {}\n // Comment with #")
# Error in rstan::stanc(model_code = "data {}\n // Comment with #") : 
#  parser failed badly; maybe try installing the V8 package

@WardBrian
Copy link
Member

I knew I wasn't crazy to suggest the # is to blame. This block of code in RStan has caused headaches for a while, and when it blows up it blames the Stan parser!

What's less clear to me is how to fix it. On more recent versions of Stan, all of the preprocessing code in RStan can just be replaced with passing an extra argument to stancjs that contains the files to-be-included, but for the version currently on CRAN some form of processing needs to be done, it just probably shouldn't invoke cpp to do it

@bgoodri
Copy link
Contributor

bgoodri commented Feb 18, 2025

Yeah, about 10 years ago I wrote the R code to walk through the lines of Stan code and resolve the #include statements. That was not entirely reliable, so somewhere along the line I thought to just outsource it to the C preprocessor because that behavior is what I was intending to replicate. But calling the preprocessor was problematic and now it is causing this CRAN issue. I agree that we shouldn't need to be doing both.

@gowerc
Copy link
Author

gowerc commented Feb 18, 2025

I knew I wasn't crazy to suggest the # is to blame. This block of code in RStan has caused headaches for a while, and when it blows up it blames the Stan parser!

What's less clear to me is how to fix it. On more recent versions of Stan, all of the preprocessing code in RStan can just be replaced with passing an extra argument to stancjs that contains the files to-be-included, but for the version currently on CRAN some form of processing needs to be done, it just probably shouldn't invoke cpp to do it

:) At the very least from the rbmi side we will update our Stan code to remove the # so at least you won't get shouted at because of us anymore 😅

@WardBrian
Copy link
Member

It does appear brms has at least one # character in it's generated Stan code:
https://github.com/paul-buerkner/brms/blob/cb2087861698a15e5f71c3bb1a1b416bfce46960/inst/chunks/fun_com_poisson.stan#L18

So this will potentially hit downstream users of brms as well

@bgoodri
Copy link
Contributor

bgoodri commented Feb 18, 2025

@paul-buerkner Can you ensure the generated Stan code has no # characters anymore. Brian found one in fun_com_poison.stan line 18? This will help us transition to a state where the parser only recognizes // and /* */ as comments.

@andrjohns
Copy link
Contributor

What's less clear to me is how to fix it. On more recent versions of Stan, all of the preprocessing code in RStan can just be replaced with passing an extra argument to stancjs that contains the files to-be-included, but for the version currently on CRAN some form of processing needs to be done, it just probably shouldn't invoke cpp to do it

@bgoodri can you make a branch with the current state of CRAN rstan? I can have a look at some fixes over the next couple days

@andrjohns
Copy link
Contributor

If I just remove that C pre-processor block from the CRAN rstan source, then all of the reverse-dependencies build and install in the C23 docker image without any issues - so it might be a simple fix after all

I'll kick off a proper reverse-dependencies run now to make sure that all of the tests/checks also still pass

@andrjohns
Copy link
Contributor

@bgoodri all revdeps pass under the C23 docker image after removving the c pre-processor block

@bernadette-eu
Copy link

bernadette-eu commented Feb 26, 2025


Dear maintainer,

Please see the problems shown on
https://cran.r-project.org/web/checks/check_results_Bernadette.html.

Please correct before 2025-03-11 to safely retain your package on CRAN.

Packages using rstan::stanc are failing to install in R-devel
using C23 (which is now the default) with current LLVM or Apple clang.
The rstan maintainer was informed 4 weeks ago and has not produced
an update (and the failure message is nonsense: V8 is installed
and that has not been checked).

The CRAN Team

Hi everyone,

I have received a similar email to @wds15 regarding my R package. The package does not pass the check in the flavors r-devel-linux-x86_64-debian-clang and r-devel-linux-x86_64-fedora-clang. I have removed # from my .stan file in /inst/stan. Is this the way to address the issue? The resubmission does not pass the incoming checks automatically.

All the best,
Lampros

@andrjohns
Copy link
Contributor

The resubmission does not pass the incoming checks automatically.

@bernadette-eu looking at the results of the resubmission checks eveything is installing and running without issue, with the only NOTE referring to your specification of arXiv references in the DESCRIPTION file:

* checking CRAN incoming feasibility ... [4s/6s] NOTE
Maintainer: ‘Lampros Bouranis <[email protected]>’

The Description field contains
  <arXiv:2211.15229>.
Please refer to arXiv e-prints via their arXiv DOI <doi:10.48550/arXiv.YYMM.NNNNN>.

@bernadette-eu
Copy link

@andrjohns Thanks for getting back to me. Let me elaborate:

Dear maintainer,

package Bernadette_1.1.6.tar.gz does not pass the incoming checks automatically, please see the following pre-tests (additional issue checks):
Windows: https://win-builder.r-project.org/incoming_pretest/Bernadette_1.1.6_20250226_125036/Windows/00check.log
Status: 1 NOTE
Debian: https://win-builder.r-project.org/incoming_pretest/Bernadette_1.1.6_20250226_125036/Debian/00check.log
Status: 1 NOTE

Last released version's CRAN status: OK: 9, NOTE: 4, ERROR: 2
See: https://cran.r-project.org/web/checks/check_results_Bernadette.html

Last released version's additional issues:
M1mac https://www.stats.ox.ac.uk/pub/bdr/M1mac/Bernadette.log
M1mac https://www.stats.ox.ac.uk/pub/bdr/M1mac/Bernadette.out

CRAN Web: https://cran.r-project.org/package=Bernadette

Please fix all problems and resubmit a fixed version via the webform.
If you are not sure how to fix the problems shown, please ask for help on the R-package-devel mailing list:
https://stat.ethz.ch/mailman/listinfo/r-package-devel
If you are fairly certain the rejection is a false positive, please reply-all to this message and explain.

More details are given in the directory:
https://win-builder.r-project.org/incoming_pretest/Bernadette_1.1.6_20250226_125036/
The files will be removed after roughly 7 days.

No strong reverse dependencies to be checked.

Best regards,
CRAN teams' auto-check service
Flavor: r-devel-linux-x86_64-debian-gcc, r-devel-windows-x86_64
Check: CRAN incoming feasibility, Result: NOTE
Maintainer: 'Lampros Bouranis <[email protected]>'

The Description field contains
arXiv:2211.15229.
Please refer to arXiv e-prints via their arXiv DOI doi:10.48550/arXiv.YYMM.NNNNN.

@andrjohns
Copy link
Contributor

andrjohns commented Feb 27, 2025

Yep, as that message says the pre-tests identified 1 NOTE in the Debian and Windows pretests, and the NOTE is copied at the bottom of the message (the arxiv reference). The other links/failures are all for the 'last released' version of your package (i.e., what is currently on CRAN)

@bgoodri
Copy link
Contributor

bgoodri commented Mar 9, 2025

OK, this looks good. I am going to run the tests on the pairs thing and this, but presumably it will all go to CRAN later today.

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

No branches or pull requests

6 participants