Skip to content
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

Fix optimum-cli command for VLM example in README #1348

Merged

Conversation

helena-intel
Copy link
Collaborator

With the existing command users get an error: Channel size 4304 should be divisible by size of group 128.

@ilya-lavrenov ilya-lavrenov added this to the 2025.0 milestone Dec 9, 2024
@ilya-lavrenov ilya-lavrenov added the port to LTS PR needs to be ported to LTS label Dec 9, 2024
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@helena-intel
Copy link
Collaborator Author

@AlexKoff88 @nikita-savelyevv on Xeon the method with --dataset, both with and without num-samples, gives an empty result with the README example. With --group-size 16 I do get a good result. Also tried to test on laptop yesterday, but there the model export did not work (unrelated to this).

@ilya-lavrenov
Copy link
Contributor

build_jenkins

@helena-intel
Copy link
Collaborator Author

helena-intel commented Dec 12, 2024

For now the commands suggested by Alexander and Nikita don't work with GenAI (see comments and linked issue). There's a ticket about that. Can anyone suggest another model that works well? Smaller is better. Otherwise I'll remove the INT4 suggestion for now, until the issue is fixed in GenAI.

@nikita-savelyevv
Copy link
Contributor

For now the commands suggested by Alexander and Nikita don't work with GenAI (see comments and linked issue). There's a ticket about that. Can anyone suggest another model that works well? Smaller is better. Otherwise I'll remove the INT4 suggestion for now, until the issue is fixed in GenAI.

OpenGVLab/InternVL2-1B is quite small and works with VLMPipeline. But I would recommend to wait for the fix huggingface/optimum-intel#1058 to be merged.

@helena-intel
Copy link
Collaborator Author

OpenGVLab/InternVL2-1B is quite small and works with VLMPipeline. But I would recommend to wait for the fix huggingface/optimum-intel#1058 to be merged.

Thank you! That's a much smaller model, much more user friendly, especially on AI PC. I tested and it works well out of the box with GenAI with the example from the README (after installing einops and timm). I'll update the PR with this. If someone disagrees, let me know.

The fix in optimum-intel is not enough for this because then exporting will work, but inference with GenAI doesn't. When that is fixed in an upcoming GenAI release we could change back to MiniCPM - though I see no downside for having smaller models in README examples.

helena-intel and others added 4 commits December 13, 2024 14:43
Co-authored-by: Alexander Kozlov <alexander.kozlov@intel.com>
Co-authored-by: Nikita Savelyev <nikita.savelyev@intel.com>
@helena-intel helena-intel force-pushed the helena/fix-vlm-optimumcli branch from 1ec6978 to 3a83895 Compare December 13, 2024 13:45
@helena-intel helena-intel changed the title Fix MiniCPM optimum-cli command in README Fix optimum-cli command for VLM example in README Dec 13, 2024
@helena-intel
Copy link
Collaborator Author

Changed README to use OpenGVLab/InternVL2-1B with INT4. With FP16/FP32 model export, inference with the README example is empty. I added it to the ticket about empty output with MiniCPM. For now, this model with INT4 works, is quick to download and convert, and it's just for an inference example in the README, INT4 should be fine.

@Wovchena Wovchena enabled auto-merge December 16, 2024 07:50
@ilya-lavrenov
Copy link
Contributor

build_jenkins

@Wovchena Wovchena added this pull request to the merge queue Dec 16, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 16, 2024
With the existing command users get an error: Channel size 4304 should
be divisible by size of group 128.

---------

Co-authored-by: Alexander Kozlov <alexander.kozlov@intel.com>
Co-authored-by: Nikita Savelyev <nikita.savelyev@intel.com>
Co-authored-by: Ilya Lavrenov <ilya.lavrenov@intel.com>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 16, 2024
@Wovchena Wovchena added this pull request to the merge queue Dec 17, 2024
Merged via the queue into openvinotoolkit:master with commit a651292 Dec 17, 2024
59 checks passed
ilya-lavrenov added a commit to ilya-lavrenov/openvino.genai that referenced this pull request Jan 5, 2025
With the existing command users get an error: Channel size 4304 should
be divisible by size of group 128.

---------

Co-authored-by: Alexander Kozlov <alexander.kozlov@intel.com>
Co-authored-by: Nikita Savelyev <nikita.savelyev@intel.com>
Co-authored-by: Ilya Lavrenov <ilya.lavrenov@intel.com>
ilya-lavrenov added a commit that referenced this pull request Jan 7, 2025
Ported:
- #1348
- #1410
- #1406
- #1424

---------

Co-authored-by: Helena Kloosterman <helena.kloosterman@intel.com>
Co-authored-by: Alexander Kozlov <alexander.kozlov@intel.com>
Co-authored-by: Nikita Savelyev <nikita.savelyev@intel.com>
Co-authored-by: Irina Efode <irina.efode@intel.com>
@ilya-lavrenov ilya-lavrenov removed the port to LTS PR needs to be ported to LTS label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants