-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
parser should flag use of _lxxf suffix on built-in functions in sampling statements #2595
Comments
Stanc3 currently says
|
This one may require a bit more care as it's a common user error. Assuming foo is real or a 1D container type, this is the right signature for normal_lpdf(foo | 0, 1). So the function with the correct signature exists, but it shouldn't be called with the "_lpdf" suffix.
Is there a way to catch this case when users include _lpmf or _lpdf by mistake and signal that they should remove the suffix? Error messages are much more useful if they suggest a fix.
… On Dec 13, 2018, at 7:41 AM, Matthijs Vákár ***@***.***> wrote:
Stanc3 currently says
Semantic error at file "test.stan", line 8, characters 2-26:
-------------------------------------------------
6: }
7: model {
8: foo ~ normal_lpdf(0, 1);
^
9: }
-------------------------------------------------
Ill-typed arguments to '~' statement. No distribution normal_lpdf was found with the correct signature.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
OK. Just added a custom error message for these cases:
|
It's "lpdf" and "lpmf" (log probability { density / mass } functions). I don't think users will be trying to use "lcdf" or "lccdf" (log {complementary / } cumulative distribution functions).
… On Dec 13, 2018, at 9:40 AM, Matthijs Vákár ***@***.***> wrote:
OK. Just added a custom error message for these cases:
Semantic error at file "test.stan", line 8, columns 8-19:
-------------------------------------------------
6: }
7: model {
8: foo ~ normal_lpdf(0, 1);
^
9: }
-------------------------------------------------
~-statement expects a distribution name without '_lpdf' or '_lcdf' suffix.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Thanks! That was definitely a typo. Fixed now. |
Fixed in stanc3. The error message is now the following:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary:
currently parser allows:
foo ~ normal_lpdf(0, 1);
due to suffix stripping logic; should flag as incorrect.Description:
parser has suffix stripping logic for user-defined probability functions, but here this applies to built-in probability distributions as well.
Reproducible Steps:
this shouldn't compile but it does:
Current Output:
currently OK; should get flagged as error.
Expected Output:
Describe what you expect the output to be. Knowing the correct behavior is also very useful.
Additional Information:
Provide any additional information here.
Current Version:
v2.18.0
The text was updated successfully, but these errors were encountered: