-
Notifications
You must be signed in to change notification settings - Fork 199
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
[JS] Add GenAI Node.js bindings #1193
base: master
Are you sure you want to change the base?
[JS] Add GenAI Node.js bindings #1193
Conversation
7784bba
to
222940d
Compare
Co-authored-by: Vladimir Zlobin <vladimir.zlobin@intel.com>
Co-authored-by: Vladimir Zlobin <vladimir.zlobin@intel.com>
Co-authored-by: Vladimir Zlobin <vladimir.zlobin@intel.com>
Co-authored-by: Vladimir Zlobin <vladimir.zlobin@intel.com>
#1193 (comment) and #1193 (comment) are left |
|
||
- name: Install build dependencies | ||
run: | | ||
sudo -E ${OPENVINO_REPO}/install_build_dependencies.sh |
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.
for other jobs in this file we re-use docker images from OpenVINO repo.
restore-keys: | | ||
${{ runner.os }}-${{ runner.arch }}-ccache-ov-and-genai-${{ matrix.build-type }} | ||
|
||
- name: Build 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.
why do we need to build them together?
As you can see, we use OpenVINO provider github actions above. So, we don't build OpenVINO anymore here.
path: ${{ env.BUILD_DIR }}/genai_nodejs_bindings.tar.gz | ||
if-no-files-found: 'error' | ||
|
||
genai_nodejs_samples_tests: |
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.
can we perform samples tests within a single JS related job which builds GenAI JS and performs all the tests ?
- name: Run tests | ||
run: npm test | ||
env: | ||
MODEL_PATH: ${{ env.JS_SRC_DIR }}/tests/models/Llama-3.2-3B-Instruct-openvino-8bit |
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 very heavy model for tests. Please, use something like katuni4ka/tiny-random-baichuan2
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, rename chat_sample folder to text_generation.
See #1411 where we move all samples from the same category to a single folder.
INCLUDES DESTINATION runtime/include) | ||
|
||
# samples do not need to be built for NPM package |
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.
samples => development files
@@ -133,13 +133,29 @@ if(MSVC OR APPLE) | |||
set(ARCH_DIR ${ARCH_DIR}/${CMAKE_BUILD_TYPE}) | |||
endif() | |||
|
|||
# Put binaries at the top level for NPM package | |||
if(CPACK_GENERATOR STREQUAL "NPM") |
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.
looks like if we follow @Wovchena we will not break anything for GenAI specifically
Adding Node.js bindings for GenAI pipelines.
Limitations
Current version it's primary backbone of future development. Supports bindings of
LLMPipeline
only.TODO