search

alashworth / test-issue-import

0 stars 0 forks source link

reject statement in generated quantities terminates execution #102

Open alashworth opened 5 years ago

alashworth commented 5 years ago

Issue by betanalpha Thursday Jun 30, 2016 at 21:40 GMT Originally opened as https://github.com/stan-dev/stan/issues/1943

Description:

The parser allows reject statements in the generated quantities block which throw uncaught exceptions causing the execution to terminate. I'm guessing this is because generated quantities is run outside of a try/catch block, but really we should just not allow reject statements outside of the model block.

Reproducible Steps:

Add reject(""); to the generated quantities block in any Stan program.

Current Output:

Execution terminates with output

Exception: Exception thrown at line [n]: 
Diagnostic information: 
Dynamic exception type: std::domain_error
std::exception::what: Exception thrown at line [n]:

Expected Output:

Ideally the parser would not have compiled the Stan program in the first place. @bob-carpenter is this easy for you to fix?

Current Version:

develop

alashworth commented 5 years ago

Comment by bob-carpenter Thursday Aug 25, 2016 at 21:25 GMT

I think this is a feature, not a bug (spoken like someone who doesn't want to deal with another issue).

Compare what happens with our built-in functions (like RNGs). They throw exceptions if you call them in generated quantities and they halt the program. If there's a user-defined function, it'll do the same thing. It seems to me like the right behavior. You can't go on if there's a rejection in the middle of evaluation.

Now I'd be OK catching the exceptions in the try-catch block. Then we can just print NaNs for all the generated quantities that haven't been evaluated in cases where there's an exception thrown during the generated quantities evaluation.

What I don't want to do is try to catch at a lower level and go on. That road leads to madness.

alashworth commented 5 years ago

Comment by bob-carpenter Thursday Aug 25, 2016 at 21:26 GMT

Oh, and the same thing happens if the constraints are violated at the end of the block for variables declared in the generated quantities block.

If you think it'd be OK to try/catch and then print NaN in these cases (maybe with yet another diagnostic flag to signal a caught exception in generated quantities), then we should close this issue and open a new one.

alashworth commented 5 years ago

Comment by betanalpha Thursday Aug 25, 2016 at 22:57 GMT

I'm fine not catching anything, but I also think that it would make sense to not allow reject() statements in the generated quantities block in the first place.

alashworth commented 5 years ago

Comment by bob-carpenter Friday Aug 26, 2016 at 02:57 GMT

But they'll get in through built-in and user-defined functions. I don't see why we'd allow them there and not allow a direct reject() statement. So how would you feel about catching and just writing out whatever the current values are? I think that also solves the problem users have been having with numerical stability when underflow or overflow in estimates or arithmetic lead to implicit rejections and halting. Maybe this should go in our next meeting agenda.