Bad line numbers when exceptions thrown in if/then tests

[betanalpha]

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:

parameters {
  real x;
}

model {
  x ~ normal(0, 1);
}

generated quantities {
  real y;

  if (x < 0) {
    y = x;
  } else if (bernoulli_rng(1.1)) {
    y = -x;
  }
}

The behavior is the same without the braces, too,

parameters {
  real x;
}

model {
  x ~ normal(0, 1);
}

generated quantities {
  real y;

  if (x < 0)
    y = x;
  else if (bernoulli_rng(1.1))
    y = -x;
}

Current Output:

Exception: Exception thrown at line 12: stan::math::bernoulli_rng: Probability parameter is 1.1, but must be between (0, 1)
Diagnostic information: 
Dynamic exception type: std::domain_error
std::exception::what: Exception thrown at line 12: stan::math::bernoulli_rng: Probability parameter is 1.1, but must be between (0, 1)

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.

[bob-carpenter]

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.

[betanalpha]

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.

[mitzimorris]

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.

[bob-carpenter]

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.

[mitzimorris]

cool!
so it's doable, provided that if-else captures start lineno for both if and else clauses.

[VMatthijs]

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.

[bob-carpenter]

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 ***@***.***> 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: <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. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

[rok-cesnovar]

Fixed with stanc3. Closing.