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

2.16 does not fix problem of missing parser errors #431

Closed
jgabry opened this issue Jul 10, 2017 · 22 comments
Closed

2.16 does not fix problem of missing parser errors #431

jgabry opened this issue Jul 10, 2017 · 22 comments
Labels
Milestone

Comments

@jgabry
Copy link
Member

jgabry commented Jul 10, 2017

Summary:

When installed from Mac binary the parser errors that were missing in v2.15 are still missing for me in v2.16. Installing from source works, but not having the parser errors when installing from binary seems like a big problem.

Reproducible Steps:

// saved in test.stan
data{
  int N; 
  real y[N];
}
parameters{
  real mu;
}
model {
  y ~ normal(mu, 1)
}
rstan::stanc(file = "test.stan")

Current Output:

> rstan::stanc(file = "test.stan")
Error in rstan::stanc(file = "Desktop/test.stan") : 
  c++ exception (unknown reason)

Expected Output:

The same parser error I would get if I had installed from source, i.e.,

> rstan::stanc(file = "test.stan")
SYNTAX ERROR, MESSAGE(S) FROM PARSER:

  error in 'model_test' at line 10, column 1
  -------------------------------------------------
     8: model {
     9:   y ~ normal(mu, 1)
        ^
  -------------------------------------------------

PARSER EXPECTED: ";"
Error in stanc(model_code = paste(program, collapse = "\n"), model_name = model_cppname,  : 
  failed to parse Stan model 'test' due to the above error.

RStan Version:

2.16.2

R Version:

3.4.1

Operating System:

OS X 10.12.5

@bob-carpenter bob-carpenter added this to the 2.15.1++ milestone Jul 10, 2017
@bob-carpenter
Copy link

It'd be great if you could patch this in RStan, as I think the error messages are super important.

Please file an issue with stan-dev/stan if you need me to fix something with the parser itself to make this possible.

The milestones aren't up to date in the issues, so I just set this to 2.15.1++.

@maverickg
Copy link
Contributor

see old issue in the previous version here #405

If you can fix it by installing from source, I would say that implies this is not a bug that we can fix. I feel like the pre-built binary version from CRAN for Mac is somehow incompatible with your local environment.

I had this problem too. But I have no way to tell what's wrong with the binary file downloaded from CRAN.

@bob-carpenter
Copy link

I just installed 2.16.2 and don't have this problem in either the GUI or RStudio:

> library(rstan)
Loading required package: ggplot2
Loading required package: StanHeaders
rstan (Version 2.16.2, packaged: 2017-07-03 09:24:58 UTC, GitRev: 2e1f913d3ca3)
For execution on a local, multicore CPU with excess RAM we recommend calling
rstan_options(auto_write = TRUE)
options(mc.cores = parallel::detectCores())
> stan_model("miss-semi.stan")
SYNTAX ERROR, MESSAGE(S) FROM PARSER:

  error in 'model105c33fb88e54_miss_semi' at line 10, column 1
  -------------------------------------------------
     8: model {
     9:   y ~ normal(mu, 1)
        ^
  -------------------------------------------------

PARSER EXPECTED: ";"
Error in stanc(file = file, model_code = model_code, model_name = model_name,  : 
  failed to parse Stan model 'miss-semi' due to the above error.
> 
> sessionInfo()
R version 3.3.2 (2016-10-31)
Platform: x86_64-apple-darwin13.4.0 (64-bit)
Running under: OS X Yosemite 10.10.5

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] rstan_2.16.2         StanHeaders_2.16.0-1 ggplot2_2.2.1       

loaded via a namespace (and not attached):
 [1] colorspace_1.3-2 scales_0.4.1     assertthat_0.1   lazyeval_0.2.0   plyr_1.8.4       tools_3.3.2     
 [7] inline_0.3.14    gtable_0.2.0     tibble_1.2       gridExtra_2.2.1  Rcpp_0.12.8      grid_3.3.2      
[13] stats4_3.3.2     munsell_0.4.3   

P.S. I would prefer to see "error in miss-semi.stan" rather than "model105c33fb88e54_miss_semi"---is there something I can do in the parser inputs/outputs to make that possible? I was assuming that the file name was passed in for the naming.

@davidaknowles
Copy link

I was having this problem (also on Mac), installing from source fixed it.

@jgabry
Copy link
Member Author

jgabry commented Sep 22, 2017 via email

@davharris
Copy link
Contributor

Looks like this is still an issue as of 2.17.2: stan-dev/stan#2458

If it can't be fixed in the next version, would it be possible to add some kind of warning for users with bad installations? Figuring out what was going on tonight took me something like a half hour.

@bob-carpenter
Copy link

Sorry this still isn't fixed. This is a critical bug that will warrant a patch release if we can fix it. I have no idea where messages are getting lost on the way from Stan to RStan.

The first thing I'd check is that everything's getting flushed on the iostream to which these messages are being sent. That kind of default can change with new implementations.

  • Works on my machine with RStan 2.17.2 installed from binaries through R GUI an RStudio on OS X v10.10.5 and Xcode 7.2.1.

  • @mitzimorris just confirmed it's still broken through R terminal on OS X v10.13.2 and Xcode v9.2. She just reinstalled 2.17.2 from binaries and the error messages are being swallowed.

@bgoodri
Copy link
Contributor

bgoodri commented Jan 15, 2018 via email

@mitzimorris
Copy link
Member

@davharris where should this warning be? put another way, where would you expect to find this information?

@mitzimorris
Copy link
Member

I've edited the RStan wiki page and added a troubleshooting section - hope this is useful.
https://github.com/stan-dev/rstan/wiki

@davharris
Copy link
Contributor

Thanks all of you for continuing to look into this.

@mitzimorris I realize that I don't know what it's like to work with CRAN on something this complicated, but here are some possibly-naive suggestions.

  • Would it be possible to use error handling/exceptions to catch the "unknown reason" error and print something more informative instead?

  • Alternatively (since there may be downsides to wrapping all calls to the parser in exception-handling code), I personally think it's a severe enough issue to justify the same sort of approach in the package's .onAttach function.

On the other hand, my experience with the bug may have been unusually bad: I'd imagine that there would be a lot more long, duplicate issue threads about this if people were routinely getting as mixed up as I did.

Anyway, thanks again.

@davharris
Copy link
Contributor

Actually, maybe I take that last bit back. I forgot about the discourse site. Looks like people have been posting about it there instead. There are over a dozen threads about "unknown reason" there, although it looks like only some of them are about this issue. It's also been raised by a few people here and in the main Stan repo.

@bob-carpenter
Copy link

bob-carpenter commented Jan 16, 2018 via email

@davharris
Copy link
Contributor

Excellent. So when your exception catcher sees c++ exception (unknown reason), it can print something like c++ exception (unknown reason): if you're seeing this message on Mac OS X with a CRAN binary, try re-installing rstan from source. Right?

@mitzimorris
Copy link
Member

a message directing folks to a specific page in the RStan wiki might be more general, although also more indirect. I've added a "Troubleshooting" section to the main page. More is needed.

@bob-carpenter
Copy link

@davharris If we could reliably print messages from exceptions, we wouldn't need to tell people to re-install from source! We're already catching the exception with the informative error message. The problem is getting error messages from our exceptions through the R plumbing when installed from CRAN.

@davharris
Copy link
Contributor

@bob-carpenter Oh, I think I see where the disconnect is: I meant R exceptions, and it sounds like maybe you meant C++ exceptions? I'll stop pestering you though. I can imagine this is very frustrating.

@bob-carpenter
Copy link

bob-carpenter commented Jan 17, 2018 via email

@davharris
Copy link
Contributor

Great. Here's a simple example of what I was trying to suggest:

This function is our "parser." It can succeed, fail, or do something weird and uninformative.

mock_parser = function(result) {
  if (result == "success") {
    return("Parsed output")
  } 
  if (result == "failure") {
    stop("Typical parser error")
  }
  if (result == "uninformative") {
    stop("something uninformative")
  }
}

Here's our exception handling (described in more detail below):

with_exceptions = function(result){
  tryCatch(
    mock_parser(result = result), 
    error = function(e){
      if (e$message == "something uninformative") {
        stop(simpleError("useful error message"))
      } else {
        stop(e)
      }
    }
  )
}

If the "parser" succeeds, our handler passes along the parsed output. If it fails, it passes along the error message, unless it's our uninformative message. In that case, it replaces the error message with something useful.

> with_exceptions("success")
[1] "Parsed output"
> with_exceptions("failure")
Error in mock_parser(result = result) : Typical parser error
> with_exceptions("uninformative")
Error: useful error message

@davharris
Copy link
Contributor

If this looks like a good approach, I'd be happy to submit a pull request this weekend, by the way.

@bob-carpenter
Copy link

I don't know R---that's up to @bgoodri, @maverickg, and @jgabry

@davidmanheim
Copy link
Contributor

Is this fixed in a more recent version, so that it can be closed?

@bgoodri bgoodri closed this as completed Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants