Callback with reload2 #2710
Merged
Callback with reload2 #2710
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atobiszei
approved these changes
Sep 24, 2024
dkalinowski
reviewed
Sep 27, 2024
Makefile
Outdated
| @@ -672,7 +672,7 @@ cpu_extension: | |||
| run_unit_tests: | |||
| ./prepare_llm_models.sh ${TEST_LLM_PATH} | |||
| ./prepare_gpu_models.sh ${GPU_MODEL_PATH} | |||
| docker run -v $(shell realpath ./rununittests.sh):/ovms/./rununittests.sh -v $(shell realpath ${GPU_MODEL_PATH}):/ovms/src/test/face_detection_adas:ro -v $(shell realpath ${TEST_LLM_PATH}):/ovms/src/test/llm_testing:ro -e https_proxy=${https_proxy} -e RUN_TESTS=1 -e JOBS=$(JOBS) -e debug_bazel_flags=${BAZEL_DEBUG_FLAGS} $(OVMS_CPP_DOCKER_IMAGE)-build:$(OVMS_CPP_IMAGE_TAG)$(IMAGE_TAG_SUFFIX) ./rununittest.sh > test.log 2>&1 ; exit_status=$$? ; tail -200 test.log ; exit $$exit_status | |||
| docker run -v $(shell realpath ./rununittests.sh):/ovms/./rununittests.sh:ro -v $(shell realpath ${GPU_MODEL_PATH}):/ovms/src/test/face_detection_adas:ro -v $(shell realpath ${TEST_LLM_PATH}):/ovms/src/test/llm_testing:ro -e https_proxy=${https_proxy} -e RUN_TESTS=1 -e JOBS=$(JOBS) -e debug_bazel_flags=${BAZEL_DEBUG_FLAGS} $(OVMS_CPP_DOCKER_IMAGE)-build:$(OVMS_CPP_IMAGE_TAG)$(IMAGE_TAG_SUFFIX) ./rununittest.sh > test.log 2>&1 ; exit_status=$$? ; tail -200 test.log ; exit $$exit_status | |||
There was a problem hiding this comment.
Why are you adding ro here? This parameter does not work at all, I have noticed it in my task. rununittests.sh does not exist. Lookup the end of the command (it does not have s):
... ./rununittest.sh > test.log ...
dkalinowski
reviewed
Sep 27, 2024
ci/cppclean.sh
Outdated
| @@ -54,7 +54,7 @@ fi | |||
| if [ ${NO_WARNINGS_TEST_NOTUSED} -gt 0 ]; then | |||
| errors+="Failed probably due to unnecessary forward includes: ${NO_WARNINGS_TEST_NOTUSED}"$'\n' | |||
| fi | |||
| if [ ${NO_WARNINGS} -gt 166 ]; then | |||
| if [ ${NO_WARNINGS} -gt 174 ]; then | |||
There was a problem hiding this comment.
Do we need to increase number of code smells?
dkalinowski
reviewed
Sep 27, 2024
dkalinowski
reviewed
Sep 27, 2024
| } catch (const std::exception& e) { | ||
| SPDLOG_DEBUG("got exception in ov::InferRequest callback: {}", e.what()); | ||
| // TODO created filter based on what is in request, then perform casual serialization for what was NOT in request, and rewrite tensors from request to response for those that were | ||
| auto status = serializePredictResponse(outputGetter, getName(), getVersion(), getOutputsInfo(), requestProto, res.get(), getTensorInfoName, useSharedOutputContentFn(requestProto)); // TODO FIXME handle status |
There was a problem hiding this comment.
@rasapala should we return error when serialization fails? why is it ignored?
There was a problem hiding this comment.
// TODO FIXME handle status - this will be done in another task.
rasapala
force-pushed
the
callback_with_reload2
branch
from
7a1736f
to
41034f4
Compare
dkalinowski
approved these changes
Sep 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA CVS-151523
Current Async implementation does not handle online modifications - we need to move unlaod guard into callback in model instance async implementation.
🧪 Checklist
``