marian-dev icon indicating copy to clipboard operation
marian-dev copied to clipboard

Fail but don't abort on user error

Open ugermann opened this issue 5 years ago • 3 comments

Currently, I think we're a bit too trigger-happy on ABORT_IF. A simple user error (such as mistyping a command line option, or missing mandatory options) should not result in a stack trace (a genuine bug in the code should, however). I propose another macro FAIL_IF that prints the error message and then exits with a non-zero exit code but doesn't trigger a stack trace, i.e.

FAIL_IF -> you made the mistake ABORT_IF -> we made the mistake

ugermann avatar Nov 01 '19 18:11 ugermann

This is related to a discussion that Marcin and I just had literally 30 minutes ago regarding throwing exceptions instead of aborting. Since Marian can be used as a library, we need a way to fail by throwing rather than aborting the process.

I like STL's separation into std::runtime_error and std::logic_error, where a runtime error depends on an external runtime condition (such as file not found, disk full, or a mal-formed JSON file), while a logic error indicates an internal inconsistency. I think we should change all ABORT calls in the Marian code to distinguish these two error types.

I think these two error types map quite well to your two classes. I.e. we could implement FAIL_IF as throwing a runtime error, and ABORT_IF as throwing a logic error. The top-level Marian main function can then catch these, but only display the call stack for logic errors.

frankseide avatar Nov 01 '19 21:11 frankseide

Sounds good to me.

ugermann avatar Nov 01 '19 21:11 ugermann

OK, this is on our TODO list. However, it will be a while, since we want to make such an intrusive change when we don't have lots of unmerged branches.

frankseide avatar Nov 01 '19 21:11 frankseide