-
Notifications
You must be signed in to change notification settings - Fork 230
Compilation with src and bin tokenizers, genai and oponvino. #3657
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.
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
Conversation
src/llm/apis/openai_completions.hpp
Outdated
| #include "absl/status/status.h" | ||
| #pragma warning(pop) | ||
| #include "../io_processing/output_parser.hpp" | ||
| #include "../generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #include "../generation_config_header.hpp" | |
| #include "src/llm/generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will be removed once GENAI fix goes into main.
| #include <utility> | ||
| #include <openvino/genai/generation_config.hpp> | ||
|
|
||
| #include "../../generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #include "../../generation_config_header.hpp" | |
| #include "src/llm/generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will be removed once GENAI fix goes into main.
| #include <openvino/genai/generation_config.hpp> | ||
|
|
||
| #include "generation_config_builder.hpp" | ||
| #include "../../generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #include "../../generation_config_header.hpp" | |
| #include "src/llm/generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will be removed once GENAI fix goes into main.
| #include <openvino/genai/generation_config.hpp> | ||
|
|
||
| #include "generation_config_builder.hpp" | ||
| #include "../../generation_config_header.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And
| #include "../../generation_config_header.hpp" | |
| #include "src/llm/generation_config_header.hpp" |
And to all other includes use non-relative path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will be removed once GENAI fix goes into main.
| WORKDIR /openvino_tokenizers/ | ||
| # Install the openvino_tokenizers python bindings and copy to OpenVINO location | ||
| RUN if ! [[ $debug_bazel_flags == *"_py_off"* ]]; then \ | ||
| RUN if [ "$ov_use_binary" == "0" ]; then true ; else exit 0 ; fi ; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea worth considering:
We could have "build_ov/genai.sh/bat" that will use variables defined in the container (stored in env.txt?).
Then in case of upgrade in OV we wouldn;t have to rebuild build container from scratch - just relaunch this script
spelling-whitelist.txt
Outdated
| @@ -1,3 +1,5 @@ | |||
| Dockerfile.redhat:228: thirdparty ==> third party, third-party | |||
| Dockerfile.ubuntu:206: thirdparty ==> third party, third-party | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's nothing like that in L206 in Ubuntu dockerfile
| load("@ovms//third_party/aws-sdk-cpp:aws-sdk-cpp.bzl", "aws_sdk_cpp") | ||
| aws_sdk_cpp() | ||
|
|
||
| ### OpenVINO GenAI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we remove this we need synchronize this with Mediapipe repository update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiles with no problem.
Makefile
Outdated
| --build-arg ov_genai_org=$(OV_GENAI_ORG)\ | ||
| --build-arg ov_tokenizers_org=$(OV_TOKENIZERS_ORG)\ | ||
| --build-arg ov_contrib_org=$(OV_CONTRIB_ORG)\ | ||
| --build-arg ov_contrib_branch=$(OV_CONTRIB_BRANCH)\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contrib is not used anymore
| sh "echo test:linux --test_env https_proxy=${env.HTTPS_PROXY} >> .user.bazelrc" | ||
| sh "echo test:linux --test_env http_proxy=${env.HTTP_PROXY} >> .user.bazelrc" | ||
| sh "make ovms_builder_image RUN_TESTS=0 OPTIMIZE_BUILDING_TESTS=1 OV_USE_BINARY=1 BASE_OS=redhat OVMS_CPP_IMAGE_TAG=${shortCommit} BUILD_IMAGE=openvino/model_server-build:${shortCommit}" | ||
| sh "make ovms_builder_image RUN_TESTS=0 OPTIMIZE_BUILDING_TESTS=1 OV_USE_BINARY=0 BASE_OS=redhat OVMS_CPP_IMAGE_TAG=${shortCommit} BUILD_IMAGE=openvino/model_server-build:${shortCommit}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 0? Shouldn't we use binary package here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not work with genai rhel8 binaries.
🛠 Summary
JIRA CVS-173265
🧪 Checklist
``