[RUNTIME] Scaffold structured error handling. by tqchen · Pull Request #2838 · apache/tvm

tqchen · 2019-03-18T00:09:13Z

Given that there is some enough interest in structured error handling #2279 and we want to have a guideline to resolve the tension of simplicity and structures. This PR attempts to provide a minimum scaffold toward that step.

Introduce tvm.error which include specific error class
Introduce the error handling convention in C++
Detect and rewrite error messages so that c++ and python stack trace are unified and not redundant.
- Remove the unnecessary stack-trace back to python(as they will be part of python trace)
- In python side, print the trace in the most recent call last order.
Introduce an initial guideline for error handling

New Error Messages

One thing that I like to highlight is that this PR allows us to simply use
LOG(FATAL) << "ErrorType: message to throw a specific error out.

We also combine the python/c++ stack trace in a reasonable way so that the error message is more human-friendly. The error message itself contains the traceback in C++ function and can be read naturally when combined with the python traceback. We also make it work for callbacks.

Example C++ code:

// src/api_test.ccvoidErrorTest(int x, int y) {
CHECK_EQ(x, y) << "ValueError: expect x and y to be equal."if (x == 1) {
LOG(FATAL) << "InternalError: cannot reach here";
}
}

The above function is registered as PackedFunc into the python frontend,
under the name tvm._api_internal._ErrorTest.
Here is what will happen if we call the registered function:

 >>> import tvm
>>> tvm._api_internal._ErrorTest(0, 1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/path/to/tvm/python/tvm/_ffi/_ctypes/function.py", line 190, in __call__
raise get_last_ffi_error()
ValueError: Traceback (most recent call last):
[bt] (3) /path/to/tvm/build/libtvm.so(TVMFuncCall+0x48) [0x7fab500b8ca8]
[bt] (2) /path/to/tvm/build/libtvm.so(+0x1c4126) [0x7fab4f7f5126]
[bt] (1) /path/to/tvm/build/libtvm.so(+0x1ba2f8) [0x7fab4f7eb2f8]
[bt] (0) /path/to/tvm/build/libtvm.so(+0x177d12) [0x7fab4f7a8d12]
File "/path/to/tvm/src/api/api_test.cc", line 80
ValueError: Check failed: x == y (0 vs. 1) : expect x and y to be equal.
>>>
>>> tvm._api_internal._ErrorTest(1, 1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/path/to/tvm/python/tvm/_ffi/_ctypes/function.py", line 190, in __call__
raise get_last_ffi_error()
tvm.error.InternalError: Traceback (most recent call last):
[bt] (3) /path/to/tvm/build/libtvm.so(TVMFuncCall+0x48) [0x7fab500b8ca8]
[bt] (2) /path/to/tvm/build/libtvm.so(+0x1c4126) [0x7fab4f7f5126]
[bt] (1) /path/to/tvm/build/libtvm.so(+0x1ba35c) [0x7fab4f7eb35c]
[bt] (0) /path/to/tvm/build/libtvm.so(+0x177d12) [0x7fab4f7a8d12]
File "/path/to/tvm/src/api/api_test.cc", line 83
InternalError: cannot reach here
TVM hint: You hit an internal error. Please open a thread on https://discuss.tvm.ai/ to report it.

It also works with recursive callbacks between python->c++->python

deftest_deep_callback():
deferror_callback():
raiseValueError("callback error")
# Implemented in c++# wrap1 = lambda : error_callback()wrap1=tvm._api_internal._test_wrap_callback(error_callback)
defflevel2():
wrap1()
wrap2=tvm._api_internal._test_wrap_callback(flevel2)
wrap2()

Traceback (most recent call last):
File "tests/python/unittest/test_runtime_error.py", line 58, in <module>
test_deep_callback()
File "tests/python/unittest/test_runtime_error.py", line 38, in test_deep_callback
wrap2()
File "/home/tqchen/github/tvm/python/tvm/_ffi/_ctypes/function.py", line 190, in __call__
raise get_last_ffi_error()
ValueError: Traceback (most recent call last):
[bt] (2) /home/tqchen/github/tvm/build/libtvm.so(TVMFuncCall+0x48) [0x7f6a422e9cb8]
[bt] (1) /home/tqchen/github/tvm/build/libtvm.so(+0x1bab28) [0x7f6a41a1cb28]
[bt] (0) /home/tqchen/github/tvm/build/libtvm.so(+0xa88ded) [0x7f6a422eaded]
File "/home/tqchen/github/tvm/python/tvm/_ffi/_ctypes/function.py", line 55, in cfun
rv = local_pyfunc(*pyargs)
File "tests/python/unittest/test_runtime_error.py", line 36, in flevel2
wrap1()
File "/home/tqchen/github/tvm/python/tvm/_ffi/_ctypes/function.py", line 190, in __call__
raise get_last_ffi_error()
[bt] (2) /home/tqchen/github/tvm/build/libtvm.so(TVMFuncCall+0x48) [0x7f6a422e9cb8]
[bt] (1) /home/tqchen/github/tvm/build/libtvm.so(+0x1bab28) [0x7f6a41a1cb28]
[bt] (0) /home/tqchen/github/tvm/build/libtvm.so(+0xa88ded) [0x7f6a422eaded]
File "/home/tqchen/github/tvm/python/tvm/_ffi/_ctypes/function.py", line 55, in cfun
rv = local_pyfunc(*pyargs)
File "tests/python/unittest/test_runtime_error.py", line 33, in error_callback
raise ValueError("callback error")
ValueError: callback error

tqchen · 2019-03-18T00:27:30Z

cc @yidawang @jennychou @junrushao1994 @jroesch @jermainewang

yidawang · 2019-03-18T00:33:09Z

Thanks @tqchen for the effort. This is amazing! I think this is a great setup for unified error messages. I particularly like the way to unify C++ and Python stack traces.

yidawang · 2019-03-18T00:35:35Z

cc @KoinFlyp @wweic

junrushao · 2019-03-18T00:36:37Z

Sounds like a plan

tqchen · 2019-03-18T00:39:19Z

see also wrapper vs less-abstraction discussion in #2279

junrushao · 2019-03-18T00:46:56Z

This is a perfect solution when all exceptions defined don't carry extra information besides their own types, and I don't think there is usecase for now that an exception would carry extra information. So I agree with the high-level idea.

Rushing for a paper deadline, will review the detailed implementation tomorrow night.

markrogersjr · 2019-03-18T18:01:34Z

+
+
+@register_error
+class OpNotImplemented(OpError):


Nice, I like how the Python and C++ error handling is unified. Should OpNotImplemented also inherit NotImplementedError? In case one is trying to catch specific error types, e.g., to gather a list of all operators or attributes that are unsupported.

markrogersjr

Looks good, just some suggestions regarding the exception hierarchy.

markrogersjr · 2019-03-18T19:24:28Z

+
+.. code:: python
+
+ def preferred():


I'm not completely sure about which of these are preferred. Is only the first option valid? Option 2 is not preferred, right?

more discussions in #2279, i think this is something that maybe worth some more discussion

markrogersjr · 2019-03-18T19:26:16Z

+
+def _valid_error_name(name):
+ """Check whether name is a valid error name."""
+ return all(x.isalnum() or x == '_' or x == '.' for x in name)


Nit: return all(x.isalnum() or x in '_.' for x in name)

markrogersjr · 2019-03-18T19:27:10Z

+
+How to choose an Error Type
+---------------------------
+You can go through the error types are listed below, try to us common


Nit: us->use

markrogersjr · 2019-03-18T19:28:07Z

+ raise OpNotImplemented("Operator relu is not implemented in the MXNet fronend")
+
+ def _op_not_implemented(op_name):
+ return OpNotImpelemented("Operator {} is not implemented.").format(op_name)


Nit: Impelemented->Implemented

markrogersjr · 2019-03-18T19:30:11Z

+
+@register_error
+class OpError(TVMError):
+ """Base class of all operator errors in fronends."""


Nit: fronends->frontends

markrogersjr · 2019-03-18T19:31:17Z

+
+
+@register_error
+class OpNotImplemented(OpError):


I suggest class OpNotImplementedError(OpError, NotImplementedError):

markrogersjr · 2019-03-18T19:31:41Z

+
+
+@register_error
+class OpAttributeRequired(OpError):


I suggest class OpAttributeRequiredError(OpError, AttributeError):

markrogersjr · 2019-03-18T19:32:13Z

+
+
+@register_error
+class OpAttributeInvalid(OpError):


I suggest class OpAttributeInvalidError(OpError, AttributeError):

markrogersjr · 2019-03-18T19:33:24Z

+
+
+@register_error
+class OpAttributeUnimplemented(OpError):


I suggest class OpAttributeNotImplementedError(OpError, AttributeError, NotImplementedError):

markrogersjr

LGTM

tqchen · 2019-03-19T06:05:22Z

Thanks, @yidawang @KoinFlyp @junrushao1994, this is now merged

[RUNTIME] Scaffold structured error handling.
928d6ab

tqchen force-pushed the error branch from a10c16b to 928d6abCompare March 18, 2019 00:25

tqchen mentioned this pull request Mar 18, 2019
Unified error handling in NNVM and Relay frontends #2828
Merged

tqchen mentioned this pull request Mar 18, 2019
[RFC] Structured Error Handling Mechanism #2279
Closed

tqchen added the status: need review label Mar 18, 2019

markrogersjr reviewed Mar 18, 2019
View reviewed changes

Update per comments
f492179

markrogersjr approved these changes Mar 19, 2019
View reviewed changes

tqchen merged commit f63631f into apache:masterMar 19, 2019

wweic pushed a commit to wweic/tvm that referenced this pull request Mar 20, 2019
[RUNTIME] Scaffold structured error handling. (apache#2838)
e945b10

wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 20, 2019
[RUNTIME] Scaffold structured error handling. (apache#2838)
017cf66

tqchen deleted the error branch October 31, 2019 17:03

tqchen mentioned this pull request Nov 8, 2019
[RELEASE][DRAFT] TVM v0.6 Release candidate #4259
Closed

tqchen mentioned this pull request Dec 16, 2019
[WINDOWS] Fix python exception Creation in Windows #4529
Closed

Uh oh!

Conversation

tqchen commented Mar 18, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Error Messages

Uh oh!

tqchen commented Mar 18, 2019

Uh oh!

yidawang commented Mar 18, 2019

Uh oh!

yidawang commented Mar 18, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junrushao commented Mar 18, 2019

Uh oh!

tqchen commented Mar 18, 2019

Uh oh!

junrushao commented Mar 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markrogersjr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markrogersjr left a comment

Choose a reason for hiding this comment

Uh oh!

tqchen commented Mar 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tqchen commented Mar 18, 2019•
edited
Loading

yidawang commented Mar 18, 2019•
edited
Loading