Cache a model, rename genai target, fix Windows #14
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
…inotoolkit#445) I got an error running benchmarking on my working machine (python3.8, ubuntu20) due to unsupported args for hashlib. ``` [ ERROR ] An exception occurred [ INFO ] Traceback (most recent call last): File "benchmark.py", line 532, in main iter_data_list, pretrain_time = CASE_TO_BENCH[model_args['use_case']](model_path, framework, args.device, model_args, args.num_iters) File "benchmark.py", line 194, in run_text_generation_benchmark run_text_generation(input_text, num, model, tokenizer, args, iter_data_list, warmup_md5, prompt_idx, bench_hook, model_precision, proc_id) File "benchmark.py", line 131, in run_text_generation result_md5_list.append(hashlib.md5(result_text.encode(), usedforsecurity=False).hexdigest()) TypeError: openssl_md5() takes at most 1 argument (2 given) ``` Based on this [StackOverflow issue](https://stackoverflow.com/questions/54717862/how-do-i-know-if-the-usedforsecurity-flag-is-supported-by-hashlib-md5), not all clients support this argument and usage hashlib.new("md5") vs hashlib.md5 should be safe for usage in both cases
) Reverts openvinotoolkit#289 to unblock the release. Since it causes the performance regression of some models. (WIP to investigate the reason)
The minimum version of transformers to get 1st and 2nd tokens latency is v4.40-release.
Co-authored-by: Chen Peter <peter.chen@intel.com>
pavel-esir
force-pushed
the
generate_pipeline
branch
from
dfb43d0
to
680e362
Compare
| cmake.build-type = "Release" | ||
| cmake.source-dir = "./" | ||
| cmake.targets = ["py_generate_pipeline"] # Adding genai would trigger a Release build and Debug build after it. py_generate_pipeline depends on genai and genai will be built anyway. It's not been investigated why both build types are triggered. |
There was a problem hiding this comment.
maybe because of ninja multi-config which builds several build types at the same time?
You can probably use the same openvinotoolkit/openvino_tokenizers#162
| - run: call w_openvino_toolkit_windows_2024.2.0.dev20240524_x86_64\setupvars.bat && python -m pip install ./thirdparty/openvino_tokenizers/[transformers] -r ./requirements-build.txt -r ./tests/python_tests/requirements.txt --extra-index-url https://storage.openvinotoolkit.org/simple/wheels/pre-release | ||
| # cmd evaluates variables in a different way. Setting PYTHONPATH before setupvars.bat instead of doing that after solves that. | ||
| - run: set "PYTHONPATH=./build/" && call w_openvino_toolkit_windows_2024.2.0.dev20240524_x86_64\setupvars.bat && python -m pytest ./tests/python_tests/test_generate_api.py --exitfirst -m precommit | ||
| - run: call w_openvino_toolkit_windows_2024.2.0.dev20240524_x86_64\setupvars.bat && python -m pip install . --config-settings=build-dir="build" --verbose |
There was a problem hiding this comment.
each line calls w_openvino_toolkit_windows_2024.2.0.dev20240524_x86_64\setupvars.bat, which make readability worse
can we use multiline script mode? I mean - run: | which assumes multiple lines after it.
| @@ -1,3 +1,5 @@ | |||
| import pathlib | |||
ilya-lavrenov
approved these changes
Jun 3, 2024
| * and unsets in destructor. Does nothing if ENVIRONMENT_VARIABLE_NAME | ||
| * was already defined. | ||
| */ | ||
| class OPENVINO_GENAI_EXPORTS ScopedVar { |
There was a problem hiding this comment.
should not be a part of public API.
You can put it into src and include src from python API
There was a problem hiding this comment.
If py_generate_pipeline is able to call tokenizers_relative_to_genai() from openvino_genai, it is a part of the API. Independently of having it in the public header.
There was a problem hiding this comment.
but this header (inside src) will not be given to users, so they will not be able to use such API (at least easily).
pavel-esir
merged commit 2c2a34a
into
pavel-esir:generate_pipeline
17 of 27 checks passed
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.
No description provided.