too bad it’s in the published paper on the BYM2 model.
agreed, I should have called it icar_normal except that I went into some detail explaining how the sampling statement works, and this is how the functions that are exposed to the language from the math library work - but now I realize I that this detail should have been explained differently. and I didn’t realize it was a bug in the compiler - I thought that it had been ruled legal.
I don’t think anybody realized what was going on given how convoluted the logic around all that name mangling with _log vs. _lpdf and using vs. not using suffixes. This only came up when I was reviewing stanc3 syntax.
That’s why I think it’s an issue of what we should do going forward. We could try to accept it and mark it as deprecated.
FWIW, the model above is the one that I’ve been presenting at talks and is in the paper on the BYM2 model, and this error wasn’t spotted by anyone - and that might include Bob.
it would be nice if the new compiler were able to provide a better error message - again, naively, I tried to change the function name as well as the function call. and/or the documentation needs to be clearer. it’s not how anything else in the language (current language) works - or is it?
I think that’s a bad idea. If “bug compatibility” is necessary, one can always resort to the older stanc. Chances are that if you “temporarily” add support for the bug, you may never get around to removing it or, even worse, introduce new bugs by attempting to remove it.
this puts everything on the user. their code which used to compile now doesn’t. there’s an error message that they don’t quite grok. and now they have to figure out how to use the old compiler.
yes it’s more work for us. this was a bug that the old compiler allowed, and IIRC, it was pretty much impossible to fix using the old bad Boost Spirit Qi. so we had to allow it. now that we can handle it, I think we should do the kindest possible thing for our users.
it’s the “replace” that’s problematic - which is implied in the message as is.
my next move was to remove the “_lpdf” from both the function definition and the function call. please understand, I was trying to fix code that I’d written 1.5 years ago - I’d forgotten all the details. this is the situation that people will be in if something changes that breaks previously working code. the user friendly thing to do is let is go for now and flag it - because not everyone has time for a teaching moment.
This isn’t a bug in the sense that it’s a bad idea and breaks things, just that it’s not our preferred syntax.
We can break backward compatibility on a major version bump, next Stan 3. But this isn’t a backward compatibility issue because it’s undocumented behavior. Nevertheless, I think we should treat it like it is as it’s come up in lots of user models, not just this one.
Ideally, when it no longer works (now or in Stan 3), we’ll have informative error messages that’d say things like “can’t use foo_lpdf(…) on right-hand side of ~, use foo(…) instead.”