Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IE PYTHON] wait for infer callback to complete #7374

Merged
merged 7 commits into from
Sep 15, 2021
Merged

[IE PYTHON] wait for infer callback to complete #7374

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ba2a45d
test commit
Sep 1, 2021
fa5798a
Add test
Sep 5, 2021
809bb8a
Remove threading.event from InferRequest
Sep 6, 2021
159e8e1
Merge remote-tracking branch 'upstream/master' into alexeyl1/ie_pytho…
Sep 7, 2021
03f643d
Fix wait for ExecutableNetwork
Sep 7, 2021
da642cd
Resolve conflicts
Sep 8, 2021
f79d649
Resolve conflicts
Sep 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion inference-engine/ie_bridges/python/src/openvino/inference_engine/ie_api.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ cdef class InferRequest:
cpdef get_perf_counts(self)
cdef void user_callback(self, int status) with gil
cdef public:
_inputs_list, _outputs_list, _py_callback, _py_data, _py_callback_used, _py_callback_called, _user_blobs
_inputs_list, _outputs_list, _py_callback, _py_data, _user_blobs

cdef class IENetwork:
cdef C.IENetwork impl
Expand Down
25 changes: 0 additions & 25 deletions inference-engine/ie_bridges/python/src/openvino/inference_engine/ie_api.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1045,14 +1045,10 @@ cdef class InferRequest:
self._inputs_list = []
self._outputs_list = []
self._py_callback = lambda *args, **kwargs: None
self._py_callback_used = False
self._py_callback_called = threading.Event()
self._py_data = None

cdef void user_callback(self, int status) with gil:
if self._py_callback:
# Set flag at first since user can call wait in callback
self._py_callback_called.set()
self._py_callback(status, self._py_data)

## Description: Sets a callback function that is called on success or failure of an asynchronous request
Expand All @@ -1076,7 +1072,6 @@ cdef class InferRequest:
def set_completion_callback(self, py_callback, py_data = None):
self._py_callback = py_callback
self._py_data = py_data
self._py_callback_used = True
deref(self.impl).setCyCallback(<cb_type> self.user_callback, <void *> self)

cpdef BlobBuffer _get_blob_buffer(self, const string & blob_name):
Expand Down Expand Up @@ -1194,8 +1189,6 @@ cdef class InferRequest:
cpdef async_infer(self, inputs=None):
if inputs is not None:
self._fill_inputs(inputs)
if self._py_callback_used:
self._py_callback_called.clear()
with nogil:
deref(self.impl).infer_async()

Expand All @@ -1215,24 +1208,6 @@ cdef class InferRequest:
cpdef wait(self, timeout=None):
cdef int status
cdef int64_t c_timeout
cdef int c_wait_mode
if self._py_callback_used:
# check request status to avoid blocking for idle requests
AlexeyLebedev1 marked this conversation as resolved.
Show resolved Hide resolved
c_wait_mode = WaitMode.STATUS_ONLY
with nogil:
status = deref(self.impl).wait(c_wait_mode)
if status != StatusCode.RESULT_NOT_READY:
return status
if not self._py_callback_called.is_set():
if timeout == WaitMode.RESULT_READY:
timeout = None
if timeout is not None:
# Convert milliseconds to seconds
timeout = float(timeout)/1000
if not self._py_callback_called.wait(timeout):
return StatusCode.REQUEST_BUSY
return StatusCode.OK

if timeout is None:
timeout = WaitMode.RESULT_READY
c_timeout = <int64_t> timeout
Expand Down
21 changes: 20 additions & 1 deletion inference-engine/ie_bridges/python/tests/test_InferRequest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import warnings
import threading
from datetime import datetime
import time

from openvino.inference_engine import ie_api as ie
from conftest import model_path, image_path
Expand Down Expand Up @@ -350,7 +351,7 @@ def execute(self, input_data):
self.cv.release()
status = self.request.wait(ie.WaitMode.RESULT_READY)
assert status == ie.StatusCode.OK
assert self.status_code == ie.StatusCode.OK
assert self.status_code == ie.StatusCode.RESULT_NOT_READY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks strange from API level perspective. User calls wait with wait mode == RESULT_READY, but receives RESULT_NOT_READY

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious about it, why we get RESULT_NOT_READY? I understand the idea behind it is to capture status inside callback which will be equal to RESULT_NOT_READY, and only after returning to python we can obtain OK. However this could render previous callbacks' "policy" absolutely obsolete. The behavior was even present around March of the current year:

openvino/inference-engine/ie_bridges/python/sample/classification_sample_async/classification_sample_async.py

Lines 24 to 29 in 6478f17

def callback(self, statusCode, userdata):
if (userdata != self.id):
log.error(f"Request ID {self.id} does not correspond to user data {userdata}")
elif statusCode != 0:
log.error(f"Request {self.id} failed with status code {statusCode}")
self.cur_iter += 1

I totally understand an idea to be able to call wait inside callback itself. However why we are still passing "the same" status code as a parameter? To my knowledge this statusCode would be equal to OK as cpp implementation marks it this way as we are able to enter a callback and not to confuse users with different names of callback's exit codes. The parameter is here:

openvino/inference-engine/ie_bridges/python/tests/test_InferRequest.py

Line 341 in 809bb8a

def callback(self, statusCode, userdata):

This needs a follow-up (in my opinion) as it requires:

  1. either reconsidering a behavior of the callbacks in python and picking of the one "true" way of obtaining the status inside the callback
  2. or changing c++ core code to apply aligned behavior in both APIs.

@nkogteva @akuporos what are your views on that?
@AlexeyLebedev1 could you elaborate more on this change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiwaszki From my point of view, we can't disallow the user to call wait inside callback and vice versa we should not force to do it just to check the status code. I agree that it is strange if status codes received in these two ways would be different. At least it should be documented.

But anyway the behavior should be aligned with C++. cc @ilya-lavrenov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main point of these changes that InferRequest.wait(RESULT_READY) must block the thread until infer_callback function completes, as in c++ api. When we call wait inside callback with no-wait status, we get RESULT_NOT_READY, also like in c++ api and it can confuse user, i agree, but if we want to wait for infer_callback maybe it makes sense.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more note: as far as I know user can access output blobs inside callback like in demo pipeline. Thus it makes sense to provide the correct status code which informs of present state - which is OK. @ilya-lavrenov is it possible to obtain results in C++ API's callbacks as well? If yes, we should consider changes to C++ API to return OK in callback or some new status that is clearer about output blobs status instead of focusing on InferRequest's "life".

If we are going to align code to C++ without any changes then it should be properly documented and reflected on the python side.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How i knew from @ilya-lavrenov , c++ api supposses that callback is the last stage of inference, that's why InferRequest.wait(0) inside callback returns RESULT_NOT_READY. I don't see why we should worry about this status code

openvino/inference-engine/ie_bridges/python/tests/test_InferRequest.py

Line 341 in f79d649

self.status_code = self.request.wait(ie.WaitMode.STATUS_ONLY)
, if we already got status code, as an argument,

openvino/inference-engine/ie_bridges/python/tests/test_InferRequest.py

Line 340 in f79d649

def callback(self, statusCode, userdata):
, which is OK, otherway we would get an exception

openvino/inference-engine/ie_bridges/python/src/openvino/inference_engine/ie_api_impl.cpp

Lines 538 to 545 in f79d649

if (code != InferenceEngine::StatusCode::OK) {
IE_EXCEPTION_SWITCH(code,
ExceptionType,
InferenceEngine::details::ThrowNow<ExceptionType>{} <<=
std::stringstream{}
<< IE_LOCATION
<< InferenceEngine::details::ExceptionTraits<ExceptionType>::string());
}
, so we can safety access output blobs inside callback.


ie_core = ie.IECore()
net = ie_core.read_network(test_net_xml, test_net_bin)
Expand All @@ -362,6 +363,24 @@ def execute(self, input_data):
del ie_core


def test_async_infer_wait_while_callback_will_not_finish(device):
def callback(status, callback_status):
time.sleep(0.5)
jiwaszki marked this conversation as resolved.
Show resolved Hide resolved
callback_status['finished'] = True

ie_core = ie.IECore()
net = ie_core.read_network(test_net_xml, test_net_bin)
exec_net = ie_core.load_network(net, device, num_requests=1)
callback_status = {}
callback_status['finished'] = False
request = exec_net.requests[0]
request.set_completion_callback(callback, py_data=callback_status)
img = read_image()
request.async_infer({'data': img})
request.wait()
assert callback_status['finished'] == True


def test_get_perf_counts(device):
ie_core = ie.IECore()
if device == "CPU":
Expand Down