-
Notifications
You must be signed in to change notification settings - Fork 172
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
Update Python VLM example in README #1178
base: master
Are you sure you want to change the base?
Conversation
Existing example uses an undefined "read_image" function, and using max_new_tokens in pipe.generate() resulted in an error with latest nightly. I updated the example to work out of the box. Makes it a bit longer, but this section is hidden by default in the README, so it doesn't add to visual clutter for people just visiting the repo.
README.md
Outdated
|
||
# Choose GPU instead of CPU in the line below to run the model on Intel integrated or discrete GPU | ||
pipe = ov_genai.VLMPipeline("/home/ubuntu/models/MiniCPM-V-2_6", "CPU") | ||
config = ov_genai.GenerationConfig() |
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.
Config creation was intentionally avoided to preserve default model generation config.
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.
The current example sets max_new_tokens in the generate() call but that returns with an error:
ValueError: 'max_new_tokens' is unexpected parameter name. Use help(openvino_genai.VLMPipeline.generate) to get list of acceptable parameters.
So what is the best practice for adding max_new_tokens without adding a config?
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.
@popovaan, please take a look why max_new_tokens
isn't accepted by VLMPipeline
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 after we have migrated to new Sampler, we have not updated a list of generation parameters accepted by Python. So, all manual parameters are ignored and we have to fix it in 2024.5
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.
Checked with 2024.5 RC4 that setting max_new_tokens directly works now so I removed the generation config from the example.
Co-authored-by: Vladimir Zlobin <[email protected]>
Existing example uses an undefined "read_image" function, and using max_new_tokens in pipe.generate() resulted in an error with latest nightly.
I updated the example to work out of the box. Makes it a bit longer, but this section is hidden by default in the README, so it doesn't add to visual clutter for people just visiting the repo. Also added links to the relevant samples.