Modernize OpenVINO-based Nuclio functions and allow them to run on Kubernetes #6129
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
SpecLad
force-pushed
the
modernize-openvino-functions
branch
2 times, most recently
from
0ac3fa6
to
96eae99
Compare
SpecLad
requested review from
azhavoro,
yasakova-anastasia and
mdacoca
as code owners
|
Converting to draft until I figure out the Helm pipeline failures. |
bsekachev
reviewed
May 12, 2023
| @@ -4,19 +4,23 @@ | |||
| SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" | |||
| FUNCTIONS_DIR=${1:-$SCRIPT_DIR} | |||
|
|
|||
| nuctl create project cvat | |||
| docker build -t cvat.openvino.base "$SCRIPT_DIR/openvino/base" | |||
There was a problem hiding this comment.
In our documentation we have some lines to build nuclio functions without deploy_cpu.sh script.
https://opencv.github.io/cvat/docs/contributing/setup-additional-components/
Don't they work anymore?
There was a problem hiding this comment.
I updated the docs. One of the nuctl deploy commands uses a TF function, which is unaffected by these changes, so I kept it as-is. The rest I replaced with calls to deploy_cpu.sh.
Comment on lines
+15
to
+20
| func_root="$(dirname "$func_config")" | ||
| func_rel_path="$(realpath --relative-to="$SCRIPT_DIR" "$(dirname "$func_root")")" |
There was a problem hiding this comment.
Should not we updated deploy_gpu.sh the same way?
There was a problem hiding this comment.
Right, thanks for the reminder. I ported relevant changes to deploy_gpu.sh.
|
|
|
|
I don't understand how that could be possible. I'm pretty sure I removed all references to
This must be a slight difference between BuildKit and the old Docker builder. I made a change to explicitly enable BuildKit - could you try again? I also made the script stop after the first error. |
Maybe it is related with #5603. I am trying to build on WSL. |
I doubt it. The only reason Nuclio would be trying to execute
I've been developing on WSL this entire time. 🤷♂️ |
SpecLad
force-pushed
the
modernize-openvino-functions
branch
from
e658952
to
7177f13
Compare
|
|
Something wrong with DEXTR masks. It should not work this way: |
|
Now all the changed functions (except the Deep extreme cut issue above) work for me. |
Why?
Could you upload the original image, so I could test it myself? |
3 tasks
…bernetes Currently, OpenVINO-based functions assume that a local directory will be mounted into the container. In Kubernetes, that isn't possible, so implement an alternate approach: create a separate base image and inherit the function image from it. In addition, implement some modernizations: * Upgrade the version of OpenVINO to the latest (2022.3). Make the necessary updates to the code. Note that 2022.1 introduced an entirely new inference API, but I haven't switched to it yet to minimize changes. * Use the runtime version of the Docker image as the base instead of the dev version. This significantly reduces the size of the final image (by ~3GB). * Replace the `faster_rcnn_inception_v2_coco` model with `faster_rcnn_inception_resnet_v2_atrous_coco`, as the former has been removed from OMZ. * Ditto with `person-reidentification-retail-0300` -> `0277`. * The IRs used in the DEXTR function are not supported by OpenVINO anymore (format too old), so rewrite the build process to create them from the original code/weights instead.
* fail early * use Docker Buildkit
…_cpu.sh I didn't migrate the changes related to `docker build` commands, since none of the GPU-based functions have separate Dockerfiles yet.
Remove the sample output for the nuclio deployment step(s), because a) it's a long wall of text that doesn't really demonstrate anything, and b) it gets even longer after recent changes, because the `docker build` commands log all build steps.
It's no longer needed, since after recent changes, the common files are baked into each image (that requires them). This also decouples our Helm chart from the rest of the repository, which I think is nice.
By default, Model Downloader downloads all available precisions.
SpecLad
force-pushed
the
modernize-openvino-functions
branch
from
7177f13
to
244e997
Compare
I think I figured it out - there were some postprocessing layers baked into the old IRs. I now added those to |
Now it works. |
Because it didn't work without explicit installing for me. |
bsekachev
approved these changes
May 15, 2023
Merged
nmanovic
added a commit
that referenced
this pull request
May 18, 2023
### Added - Introduced a new configuration option for controlling the invocation of Nuclio functions. (<#6146>) ### Changed - Relocated SAM masks decoder to frontend operation. (<#6019>) - Switched `person-reidentification-retail-0300` and `faster_rcnn_inception_v2_coco` Nuclio functions with `person-reidentification-retail-0277` and `faster_rcnn_inception_resnet_v2_atrous_coco` respectively. (<#6129>) - Upgraded OpenVINO-based Nuclio functions to utilize the OpenVINO 2022.3 runtime. (<#6129>) ### Fixed - Resolved issues with tracking multiple objects (30 and more) using the TransT tracker. (<#6073>) - Addressed azure.core.exceptions.ResourceExistsError: The specified blob already exists. (<#6082>) - Corrected image scaling issues when transitioning between images of different resolutions. (<#6081>) - Fixed inaccurate reporting of completed job counts. (<#6098>) - Allowed OpenVINO-based Nuclio functions to be deployed to Kubernetes. (<#6129>) - Improved skeleton size checks after drawing. (<#6156>) - Fixed HRNet CPU serverless function. (<#6150>) - Prevented sending of empty list of events. (<#6154>)
mikhail-treskin
pushed a commit
to retailnext/cvat
that referenced
this pull request
Jul 1, 2023
…bernetes (cvat-ai#6129) Currently, OpenVINO-based functions assume that a local directory will be mounted into the container. In Kubernetes, that isn't possible, so implement an alternate approach: create a separate base image and inherit the function image from it. In addition, implement some modernizations: * Upgrade the version of OpenVINO to the latest (2022.3). Make the necessary updates to the code. Note that 2022.1 introduced an entirely new inference API, but I haven't switched to it yet to minimize changes. * Use the runtime version of the Docker image as the base instead of the dev version. This significantly reduces the size of the final image (by ~3GB). * Replace the `faster_rcnn_inception_v2_coco` model with `faster_rcnn_inception_resnet_v2_atrous_coco`, as the former has been removed from OMZ. * Ditto with `person-reidentification-retail-0300` -> `0277`. * The IRs used in the DEXTR function are not supported by OpenVINO anymore (format too old), so rewrite the build process to create them from the original code/weights instead.
|
Hi, @SpecLad does this PR mean, that I could run SAM via cvat helm chart? Or that is not possible right now? I tried to check the docs and do not see any info about how to do that. |
|
@Keramblock You cannot deploy serverless functions themselves using the Helm chart, but you can deploy Nuclio with the chart (set the EDIT: Also, I should note that this PR doesn't affect SAM, as SAM is not OpenVINO-based. |
|
Oh, ok, thanks a lot for the clarification! |
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.
Motivation and context
Currently, OpenVINO-based functions assume that a local directory will be mounted into the container. In Kubernetes, that isn't possible, so implement an alternate approach: create a separate base image and inherit the function image from it.
In addition, implement some modernizations:
Upgrade the version of OpenVINO to the latest (2022.3). Make the necessary updates to the code. Note that 2022.1 introduced an entirely new inference API, but I haven't switched to it yet to minimize changes.
Use the runtime version of the Docker image as the base instead of the dev version. This significantly reduces the size of the final image (by ~3GB).
Replace the
faster_rcnn_inception_v2_cocomodel withfaster_rcnn_inception_resnet_v2_atrous_coco, as the former has beenremoved from OMZ.
Ditto with
person-reidentification-retail-0300->0277.The IRs used in the DEXTR function are not supported by OpenVINO anymore (format too old), so rewrite the build process to create them from the original code/weights instead.
How has this been tested?
I manually tried each affected function to make sure they still work.
Checklist
developbranch[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes[ ] I have linked related issues (see GitHub docs)[ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.