-
Notifications
You must be signed in to change notification settings - Fork 302
[GENAI]Introduce add_extension to genai. #2952
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
Open
xipingyan
wants to merge
43
commits into
openvinotoolkit:master
Choose a base branch
from
xipingyan:xp/introduce_add_extention_to_genai
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+74
−7
Open
Changes from 11 commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
7b3ad31
Introduce add_extension to genai.
xipingyan 2f60301
add draft test.
xipingyan 80efe54
Update src/python/openvino_genai/py_openvino_genai.pyi
xipingyan 4a02c8e
Update src/python/openvino_genai/__init__.pyi
xipingyan 31078ab
Update tests/python_tests/test_add_extension.py
xipingyan 281fe8c
Update tests/python_tests/test_add_extension.py
xipingyan ad64976
Merge branch 'master' into xp/introduce_add_extention_to_genai
xipingyan e449070
Update src/python/openvino_genai/py_openvino_genai.pyi
xipingyan 4f60a60
Update src/cpp/include/openvino/genai/generation_config.hpp
xipingyan 698e5ec
Update generation_config.hpp
xipingyan 8c56ca8
Update tests/python_tests/test_add_extension.py
xipingyan c1e0a7c
Update src/python/py_openvino_genai.cpp
xipingyan 57bd6a1
Update tests/python_tests/test_add_extension.py
xipingyan a4ef9c8
Update tests/python_tests/test_add_extension.py
xipingyan 7260493
Update src/cpp/include/openvino/genai/generation_config.hpp
xipingyan 689b08c
Get tokenizer so path.
xipingyan 12e53a1
Merge branch 'master' into xp/introduce_add_extention_to_genai
xipingyan 072b0b0
Update tests/python_tests/test_add_extension.py
xipingyan c05b092
Update tests/python_tests/test_add_extension.py
xipingyan 7e1ac0f
Update tests/python_tests/test_add_extension.py
xipingyan ba86cc4
enable add_extension to vlm pipleline with properties.
xipingyan 4705288
Merge remote-tracking branch 'origin/xp/introduce_add_extention_to_ge…
xipingyan 323e4c2
remove unecessary file
xipingyan 82b65bc
Update src/cpp/src/utils.cpp
xipingyan 3b6391d
Update src/cpp/src/visual_language/pipeline.cpp
xipingyan 0542ae3
Update tests/python_tests/test_vlm_pipeline.py
xipingyan 02d5310
Update tests/python_tests/test_vlm_pipeline.py
xipingyan 97a88fc
Update src/cpp/src/visual_language/pipeline.cpp
xipingyan 200fb05
Remove print.
xipingyan f49ebb9
Update src/cpp/include/openvino/genai/generation_config.hpp
xipingyan 611c1fb
git pushMerge branch 'xp/introduce_add_extention_to_genai' of https:/…
xipingyan 34499ec
enable llm piple , lots of interleave call, about ContinuousBatchingP…
xipingyan a92fe1f
add test case of cb and llm
sunxiaoxia2022 75900d0
change extract_extensions to add_extensions_to_core
sunxiaoxia2022 bf6bd7f
fix conflict
sunxiaoxia2022 8a80895
revert some change
sunxiaoxia2022 85608a0
rm add_extensions_to_core from pipeline_stateful
sunxiaoxia2022 6fd6b26
revert a small change
sunxiaoxia2022 3208ed1
revert a change
sunxiaoxia2022 6e10e60
double quotes
sunxiaoxia2022 6e673c9
rm add_extensions_to_core from embedding_model and qwen2vl due to bei…
sunxiaoxia2022 e59ab7c
move add_extensions_to_core before InputsEmbedder
sunxiaoxia2022 54880e3
add a documentation comment for add_extensions_to_core
sunxiaoxia2022 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # Copyright (C) 2023-2025 Intel Corporation | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
|
|
||
| import pytest | ||
| import openvino_genai as ov_genai | ||
|
|
||
| @pytest.mark.precommit | ||
| def test_add_extension(): | ||
| print(ov_genai.get_version()) | ||
xipingyan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # Path to the OpenVINO extension shared library (update as needed). | ||
| extension_path = "" | ||
| with pytest.raises(Exception): | ||
xipingyan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ov_genai.add_extension(extension_path) | ||
Oops, something went wrong.
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.
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.
Please take a different approach.
Introduce a new
ov::Property<std::vector<std::string>>that will correspond toov::Core::add_extension(). If this property is given in pipeline constructor, pass its content to the correspondingov::Core.ov::Coreshould no longer be singleton. Any pipeline that introduces support foradd_extension()should manage its ownov::Core. This requires openvinotoolkit/openvino#32738 to be merged. To allow otherov::Core's functions in future, GenAI should introduce a private common function that calls the neededov::Core's function given properties (it should be onlyadd_extension()at this moment). Every pipeline needs to call this helper.Don't modify all pipelines at once. Split the changes into multiple PRs.
Let me know if you have other though
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.
I list all candidates:
So I will prioritize to enable
LLMPipelineandVLMPipelinein the first PR.Other question about test case,
Actually, I don't know how to add appropriate test case, because we need to pass a library(dll or so) path.
What do you think about current enabled test case? Only pass tokenizer library path and check if we can catch any exceptions. @Wovchena
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 idea for test is nice. I can't come up with a different test case
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.
We can't get any exception when we pass correct tokenizer library, so I think we can pass a fake path, we can get openvino threw exception log, just check this log, it may be better.
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.
What log are you referring to?
looks correct
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.
because we pass faked path, so openvino will throw exception with specific log, I just catch this exception and compare this log. so this log is from openvino, if openvino update the threw log, maybe trigger this test fail.
is this reasonable? @Wovchena @sunxiaoxia2022 @peterchen-intel
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.
It's reasonable. There seem to be no other way
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 going to be a discussion with arch team this Monday. I'll update you after that
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.
Moved to Wednesday