-
Notifications
You must be signed in to change notification settings - Fork 397
Update Phi3 Android App #520
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
Conversation
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.
Pull Request Overview
This PR upgrades the Android example to ONNX Runtime GenAI v0.8.x, refactors MainActivity to use the new SimpleGenAI API with a generic model configuration, and updates build scripts and documentation.
- Bump ONNX Runtime GenAI AAR to 0.8.x and Gradle wrapper to 8.9 with added timeout/validation
- Refactor
MainActivityto useSimpleGenAI, atomic counters, and a configurable model download - Overhaul Gradle start scripts (
gradlew,gradlew.bat) with SPDX license headers, improved error redirection, and POSIX-compliant parsing
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| mobile/examples/phi-3/android/gradlew.bat | Added SPDX license, unified error redirection, and robust exit code |
| mobile/examples/phi-3/android/gradlew | Rewritten POSIX script: updated shebang, symlink resolution, arg parsing, use of xargs |
| mobile/examples/phi-3/android/gradle/wrapper/gradle-wrapper.properties | Upgraded Gradle distribution to 8.9, added networkTimeout and validateDistributionUrl |
| mobile/examples/phi-3/android/app/src/main/java/ai/onnxruntime/genai/demo/MainActivity.java | Switched to SimpleGenAI API, introduced generic model constants, atomic counters, streaming listener |
| mobile/examples/phi-3/android/app/build.gradle.kts | Updated GenAI AAR from 0.4.0-dev to 0.8.1 |
| mobile/examples/phi-3/android/README.md | Expanded README with generic model instructions and updated setup steps |
Comments suppressed due to low confidence (1)
mobile/examples/phi-3/android/app/src/main/java/ai/onnxruntime/genai/demo/MainActivity.java:23
- Unused import detected; consider removing
ExecutorService(andExecutorson the next line) to clean up dependencies.
import java.util.concurrent.ExecutorService;
mobile/examples/phi-3/android/app/src/main/java/ai/onnxruntime/genai/demo/MainActivity.java
Outdated
Show resolved
Hide resolved
| // ONNX Runtime with GenAI | ||
| implementation("com.microsoft.onnxruntime:onnxruntime-android:latest.release") | ||
| implementation(files("libs/onnxruntime-genai-android-0.4.0-dev.aar")) | ||
| implementation(files("libs/onnxruntime-genai-android-0.8.1.aar")) |
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.
is there a published onnxruntime-genai-android package that we can use?
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 downloaded the aar file from https://github.com/microsoft/onnxruntime-genai/releases. GenAI is not publishing android packages on maven
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.
wonder if there's a way to have gradle download from github? it'd be nice to not have to keep a copy of the .aar in the repo. we don't have to figure that out in this PR though.
mobile/examples/phi-3/android/app/src/main/java/ai/onnxruntime/genai/demo/MainActivity.java
Outdated
Show resolved
Hide resolved
…/genai/demo/MainActivity.java Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com>
| AtomicLong firstTokenTime = new AtomicLong(startTime); | ||
| AtomicInteger numTokens = new AtomicInteger(0); |
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 believe these don't need to be atomic. now, the SimpleGenAI documentation says that the listener will not be called concurrently.
https://github.com/microsoft/onnxruntime-genai/blob/0a34c76e024cff85f0018f495e67de0bda417ffe/src/java/src/main/java/ai/onnxruntime/genai/SimpleGenAI.java#L61-L64
Update the ONNX Runtime GenAI to
v0.8.1and refactor MainActivity to use SimpleGenAI API.Fixes: #511