Support populating errors back to MXNet engine in callback #13922
Conversation
include/mxnet/engine.h
Outdated
| @@ -74,15 +74,15 @@ class CallbackOnComplete { | |||
| public: | |||
| // use implicit copy and assign | |||
| /*! \brief involve the callback */ | |||
| inline void operator()() const { | |||
| (*callback_)(engine_, param_); | |||
| inline void operator()(const char* error_msg = nullptr) const { | |||
There was a problem hiding this comment.
Instead of passing in a const string of error message, would it make sense to pass in a error struct which contains error message and error code in the fields? This leaves more room for proper error handling based on error code.
There was a problem hiding this comment.
The error code is not universally defined across different libraries. In the Horovod case, the error types are mostly Horovod specific. We convert all those to dmlc::Error which MXNet can catch.
There was a problem hiding this comment.
Can we define an error data structure and export it? So all libraries that consume libmxnet will use this error data structure to pass data back.
There was a problem hiding this comment.
This sounds like a good idea to make it extendable in the future. We can use the error message for now.
There was a problem hiding this comment.
Use dmlc::Error instead of char*. We can change dmlc::Error if more information need to be passed.
|
Nice work @yuxihu |
|
@eric-haibin-lin please take a look. |
| OprBlock *opr_block = static_cast<OprBlock*>(opr_block_); | ||
| ThreadedOpr *threaded_opr = opr_block->opr; | ||
| if (error != nullptr) { | ||
| auto ex_p = std::make_exception_ptr(*error); | ||
| threaded_opr->opr_exception = std::make_shared<std::exception_ptr>(ex_p); |
There was a problem hiding this comment.
Isn't exception_ptr already reference counted? Do we need to wrap it again in make_shared?
There was a problem hiding this comment.
Good catch. I will change related sites by removing the redundant shared_ptr in a separate PR.
This PR adds an optional dmlc::Error* argument in MXNet engine callback functions. The callers can leverage this argument to populate errors back to MXNet engine through callback such that the errors can be handled properly by MXNet engine. One such use case is to populate the errors detected in Horovod back to MXNet engine for Hovorod and MXNet integration. This change does not affect existing use cases.