Open alashworth opened 5 years ago
Comment by bob-carpenter
Thursday Aug 25, 2016 at 21:19 GMT
Yes, it's only when the exceptions come from the checks themselves. That one's hard to fix, because only the statements have line numbers attached to them and the conditional expressions aren't statements.
You can see that in the generated code where there are line number increment statements everywhere (it's a rather crude and run-time intensive solution). And the if-then is one big statement; there isn't a statement to execute if the first conditional expression fails in an if-else.
I could update the messages so they indicated a range of line numbers because I store the start and ending line for a statement---that might be less confusing.
Comment by betanalpha
Thursday Aug 25, 2016 at 22:55 GMT
That would be a reasonable solution. It’s just confusing when one assumes a 1-1 relationship between the error and the line number.
On Aug 25, 2016, at 2:20 PM, Bob Carpenter notifications@github.com wrote:
Yes, it's only when the exceptions come from the checks themselves. That one's hard to fix, because only the statements have line numbers attached to them and the conditional expressions aren't statements.
You can see that in the generated code where there are line number increment statements everywhere (it's a rather crude and run-time intensive solution). And the if-then is one big statement; there isn't a statement to execute if the first conditional expression fails in an if-else.
I could update the messages so they indicated a range of line numbers because I store the start and ending line for a statement---that might be less confusing.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
Comment by mitzimorris
Sunday Apr 29, 2018 at 15:24 GMT
here is the generated code for the 2nd example:
// generated quantities statements
current_statement_begin__ = 12;
if (as_bool(logical_lt(x, 0))) {
current_statement_begin__ = 13;
stan::math::assign(y, x);
} else if (as_bool(bernoulli_rng(1.1, base_rng__))) {
current_statement_begin__ = 15;
stan::math::assign(y, -(x));
}
in order to make this fly, the parser would have to capture the start and end lines of the if
and else
clauses separately, and the generated code would be something like this:
// generated quantities statements
current_statement_begin__ = 12;
if (as_bool(logical_lt(x, 0))) {
current_statement_begin__ = 13;
stan::math::assign(y, x);
} else {
current_statement_begin__ = 14;
if (as_bool(bernoulli_rng(1.1, base_rng__))) {
current_statement_begin__ = 15;
stan::math::assign(y, -(x));
}
}
doable, but a real PITA when working in the boost::spirit::qi parser.
Comment by bob-carpenter
Sunday Apr 29, 2018 at 18:38 GMT
That's still not quite enough if it ends in the middle of an else if.
if (cond1) ...
else if (cond2) ...
The problem is that there's no hook within the body to increment before evaluating cond2
. So it'd have to be something like this:
if (cond1) ...
else if (current-statement_begin__ = 19, cond2) ...
In general, (a, b)
is an expression that evaluates a
, then evaluates and returns b
. Any statement can be used as an expression, so the assignment is OK as the first expression here. And because it can't fail, the condition evaluates to the same thing as cond2
.
Comment by mitzimorris
Sunday Apr 29, 2018 at 18:47 GMT
cool! so it's doable, provided that if-else captures start lineno for both if and else clauses.
Comment by VMatthijs
Thursday Dec 13, 2018 at 13:14 GMT
Observe that this even happens with static errors, rather than runtime errors. For example, on the ill-formed model
parameters {
real x;
}
model {
x ~ normal(0, 1);
}
generated quantities {
real y;
if (x < 0) {
y = x;
} else if ({1,2}) {
y = -x;
}
}
stanc2 says
SYNTAX ERROR, MESSAGE(S) FROM PARSER:
Conditions in if-else statement must be primitive int or real; found type=int[ ]
error in 'stan/src/test/test-models/good/empty.stan' at line 11, column 14
-------------------------------------------------
9: generated quantities {
10: real y;
11:
^
12: if (x < 0) {
-------------------------------------------------
PARSER EXPECTED: <expression>
Instead, stanc3 now says
Semantic error at file "test.stan", line 14, characters 13-18:
-------------------------------------------------
12: if (x < 0) {
13: y = x;
14: } else if ({1,2}) {
^
15: y = -x;
16: }
-------------------------------------------------
Condition in conditional needs to be of type int or real. Instead found type int[].
I hope we can also translate this into a better error location for runtime errors.
Comment by bob-carpenter
Thursday Dec 13, 2018 at 14:08 GMT
That's great. We never got line number identifiers for expressions going everywhere, so couldn't isolate the error in stanc2.
On Dec 13, 2018, at 8:14 AM, Matthijs Vákár notifications@github.com wrote:
Observe that this even happens with static errors, rather than runtime errors. For example, on the ill-formed model
parameters { real x; }
model { x ~ normal(0, 1); }
generated quantities { real y;
if (x < 0) { y = x; } else if ({1,2}) { y = -x; } }
stanc2 says
SYNTAX ERROR, MESSAGE(S) FROM PARSER: Conditions in if-else statement must be primitive int or real; found type=int[ ] error in 'stan/src/test/test-models/good/empty.stan' at line 11, column 14
9: generated quantities { 10: real y; 11: ^ 12: if (x < 0) {
PARSER EXPECTED:
Instead, stanc3 now says
Semantic error at file "test.stan", line 14, characters 13-18:
12: if (x < 0) { 13: y = x; 14: } else if ({1,2}) { ^ 15: y = -x; 16: }
Condition in conditional needs to be of type int or real. Instead found type int[].
I hope we can also translate this into a better error location for runtime errors.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.
Description:
When exceptions are thrown in the logic of if/then statements the line number points to the first if statement and not the actual line where the exception was encountered.
Reproducible Steps:
The behavior is the same without the braces, too,
Current Output:
Expected Output:
The exception is thrown at line 14. Line 12 is where the if statement begins.
Additional Information:
Exceptions thrown within the body of the if/then statements report accurate line numbers. It just seems to be when the exceptions are thrown in the checks themselves.
Current Version:
Develop.