-
Notifications
You must be signed in to change notification settings - Fork 28
Add QNN support for Gemma3-4b-it #125
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
base: main
Are you sure you want to change the base?
Conversation
| def run(args: argparse.Namespace): | ||
| logger.info("Loading model...") | ||
| config = og.Config(args.model_path) | ||
| if args.execution_provider != "follow_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.
should we be providing some error/warning messages if either of these conditions are not true
| if args.image_paths: | ||
| image_paths = args.image_paths | ||
| else: | ||
| image_paths = [str(Path(__file__).parent / "images" / "dog.jpg")] |
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.
do we want to guarantee that this exists? or should we just require args.image_paths in non-interactive
| if args.prompt: | ||
| text = args.prompt | ||
| else: | ||
| text = "What is shown in this image?" |
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.
same comment here regarding default vs. requiring args.prompt
|
|
||
| # Construct the "messages" argument passed to apply_chat_template | ||
| messages = [] | ||
| if model.type == "phi3v": |
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.
should we remove the phi3v version of things since this is the gemma example?
| uv pip install setuptools | ||
|
|
||
| # Requires installation of uv | ||
| uv pip install -r ../requirements.txt |
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 need to look at qnn_req.txt and confirm whether that needs to be added here
| @@ -0,0 +1,379 @@ | |||
| { | |||
| "cells": [ | |||
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'm a little hesitant to say we need both the notebook and the README.md/env_setup.sh script
maybe we can look to consolidate things so that we don't have duplicate commands in multiple places and have to essentially maintain multiple copies of the same instructions (e.g. remove some commands from README, have the notebook just call the env_setup.sh script for some stuff, etc.)
|
|
||
| - **`env_setup.sh`** - Automated environment setup script (Linux only) | ||
| - **`gemma3-4b-text-qnn-config.json`** - Olive configuration for optimizing the text component | ||
| - **`gemma3-4b-vision-qnn-config.json`** - Olive configuration for optimizing the vision component |
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.
need to add additional files here
| @@ -0,0 +1,7 @@ | |||
| coloredlogs | |||
| flatbuffers | |||
| numpy >= 1.21.6 | |||
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.
need to look at whether we can just move these into requirements.txt?
|
Please add a description to the PR and resolve all open conversations. |
e0ff1ef to
e81cb34
Compare
No description provided.