Stancjs: Check input JS types before conversion #1475
Conversation
All either ICE or cause a JS exception
WardBrian
added
cleanup
There was a problem hiding this comment.
Closes #1446 by improving the error message.
Will this stanc3 change go out before RStan gets fixed? If not then technically users won't see the new error message and it was the RStan fix that closed issue 1446. If yes, then I think it would be worthwhile to try to fix the issue here as well. If it's just RStan passing array-of-strings instead of a single string, then concatenating the strings should be easy.
The RStan experimental branch picks up nightly, but this issue is present in the current CRAN release (based on Stan 2.33), so it really has to be fixed by them for most users to benefit, since updating to a more recent version will take another long while I assume. RStan’s string preprocessing of the model code is broken in at least 3 ways right now, and I don’t think the compiler can or should take on the burden of trying to fix it… Anyway, I think technically finding the cause and opening the RStan issue is the thing that resolves 1446 from the compilers perspective. This is mostly just trying to play a little nicer (and trying to cut down on the noise in the issue list, since it will no longer give a link to open one here) |
|
I also think we should probably merge this after the release, just because it does touch a lot of the stancjs driver |
Closes #1446 by improving the error message. Note that the actual bad behavior is in RStan, c.f. stan-dev/rstan#1145
Submission Checklist
Release notes
stanc.js will now properly complain if it is supplied with bad javascript types for its arguments.
Copyright and Licensing
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)