Closed WardBrian closed 4 months ago
This is failing downstream due to https://github.com/stan-dev/math/issues/1039: Some CVODES errors are still thrown as generic runtime_error
s in https://github.com/stan-dev/math/blob/develop/stan/math/prim/err/check_flag_sundials.hpp
@syclik - the discussion from back then suggested these becoming domain_error
s, but they didn't. Was there some reason for this, or should I go ahead and open a PR to math which does that?
@SteveBronder mind giving this a look?
std::out_of_range
would be considered fatal by this and lead to the algorithms terminating. Is there something else you're looking for clarification on about them?
out of range can make sense, but what about std::invalid_argument
? Like quad_form_sym
would throw that for symmetric matrices. Would we just fail on that error and end the program?
Those have been considered unrecoverable (at least during initialization) for a while. invalid_argument
is thrown for things like check_square
, which won't change if you just 'try again'.
I think if there are usages in Math we want to be recoverable, we should change those to match the others
Okay looking at this again I think this is fine to merge 👍
Submission Checklist
./runTests.py src/test/unit
make cpplint
Summary
Closes #3258. This changes any place which caught
std::exception
from the model to only catchstd::domain_error
, matching what is done during initialization.This prevents odd behavior like an indexing exception that only occurs randomly being swallowed if initialization happens to pass, and allows things like an
exit()
function in the language.Intended Effect
Cause certain exceptions to terminate the inference algorithms.
How to Verify
This model will usually initialize but quickly fail now:
Side Effects
I've installed handlers in the service functions, but code that calls the internals of these algorithms (e.g
run_adaptive_sampler
) not through the services will need to be ready to catch exceptionsAdditionally, if any Math functions raise an exception which is intended to be recoverable but is not a domain_error, they will need updating. Because this is the current behavior in initialization, I suspect we have probably already smoked any of these out.
Documentation
Copyright and Licensing
Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):
Simons Foundation
By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses: