-
Notifications
You must be signed in to change notification settings - Fork 5
CIFAR 10 ExecuTorch model fine-tuning on the Android app #41
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?
CIFAR 10 ExecuTorch model fine-tuning on the Android app #41
Conversation
…iles in Cifar10ImageExtractor.kt file
…with newest version of ExecuTorch aar
…n the official ExecuTorch repo
@IshanAryendu I think we should follow the current pattern of the repo and put this under |
cifar/README.md
Outdated
|
||
For the android environment setup, follow these steps: | ||
|
||
1. Install Open JDK using: `brew install openjdk@17` |
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.
Not everyone will be using macOS. Let's just delegate the instruction to OpenJDK. However, I also think we don't really need to do this — it might be safe to assume Android engineers already know about this (+ the other instructions related to Android in general below)
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'll update this to direct the people to OpenJDK and follow the steps over there to configure their environment
cifar/README.md
Outdated
7. Export the paths for these to your environment or add them to the `.bashrc` or `.zshrc` files: | ||
```bash | ||
# JAVA config | ||
export JAVA_HOME=/opt/homebrew/opt/openjdk@17/ |
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.
This is macOS and version specific, we shouldn't include that. We can reduce these steps to:
Ensure the following environment variables are set: `JAVA_HOME`, `ANDROID_NDK`, `ANDROID_SDK`
Better yet we can enforce that in our build scripts ;)
} | ||
return file.absolutePath | ||
} catch (e: IOException) { | ||
Log.e("AssetCopy", "Error copying asset $assetName: ${e.message}") |
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.
Why this tag instead of ExecuTorchApp
?
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.
It was there to track if the files are properly copied. I've replaced it with the debugTag.
FileOutputStream(file).use { out -> | ||
bitmap.compress(Bitmap.CompressFormat.PNG, 100, out) | ||
} | ||
Log.d("ExecuTorchApp", "Saved image to ${file.absolutePath}") |
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.
Can you extract this tag out as a variable? If we need to change it in the future it would be much easier.
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 variable debugTag stores the required tag for easier maintenance
} | ||
|
||
trainImagesLoaded += imagesToLoad | ||
Log.d("Cifar10Loader", "Total training images loaded so far: $trainImagesLoaded") |
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.
inconsistent tag
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.
Fixed the inconsistency
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.
@psiddh can you also take a look?
cifar/README.md
Outdated
|
||
**Note:** We will rename this file to `executorch.aar` and copy it into the `libs` directory of the android app. | ||
|
||
## Creation of Android App |
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 really need this step ? The current app layout has all the necessary files & structure. Can we remove this section ? that way it can be less verbose
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.
Made the recommended changes to the README file
```bash | ||
$ sh ./scripts/build_android_library.sh | ||
$ sh ./extension/android/executorch_android/android_test_setup.sh | ||
$ ls ./extension/android/executorch_android/ | ||
$ cd extension/android | ||
$ ./gradlew :executorch_android:testDebugUnitTest | ||
$ ./gradlew :executorch_android:connectedAndroidTest | ||
``` |
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 really need to do this?
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've retained these steps as a sanity check
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.
Sanity check for who/what?
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.
Oh, these are used to run the unit tests and instrumented tests for the newly built modules
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 should assume that the tests already pass — we don't run tests for libraries we use. If a user skips these tests, will the training example work? If so, let's remove it
cifar/android/CifarETTrainingDemo/app/src/test/java/com/example/democifar10/ExampleUnitTest.kt
Outdated
Show resolved
Hide resolved
cifar/android/CifarETTrainingDemo/app/src/main/res/xml/data_extraction_rules.xml
Outdated
Show resolved
Hide resolved
cifar/android/CifarETTrainingDemo/app/src/main/res/xml/backup_rules.xml
Outdated
Show resolved
Hide resolved
cifar/android/CifarETTrainingDemo/app/src/main/java/com/example/democifar10/ui/theme/Type.kt
Outdated
Show resolved
Hide resolved
cifar/android/CifarETTrainingDemo/app/src/main/java/com/example/democifar10/ui/theme/Theme.kt
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,10 @@ | |||
<?xml version="1.0" encoding="utf-8"?> |
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 need this?
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.
seems like Android standard
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.
oh maybe it's optional, since strings.xml
doesn't have it?
```bash | ||
$ sh ./scripts/build_android_library.sh | ||
$ sh ./extension/android/executorch_android/android_test_setup.sh | ||
$ ls ./extension/android/executorch_android/ | ||
$ cd extension/android | ||
$ ./gradlew :executorch_android:testDebugUnitTest | ||
$ ./gradlew :executorch_android:connectedAndroidTest | ||
``` |
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 should assume that the tests already pass — we don't run tests for libraries we use. If a user skips these tests, will the training example work? If so, let's remove it
Please address @jathu's comments |
…and copy the outputs to the destination.
|
||
## Creation of Android App | ||
|
||
1. Start with a clone of this repository and open the project in Android Studio. |
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.
Much cleaner now, can add path to gradle file ?
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.
This file: cifar/android/CifarETTrainingDemo/app/build.gradle.kts
?
In this tutorial, we are trying to fine-tuning a CIFAR 10 model on an Android device using ExecuTorch PTE + PTD combo. This example is an extension of the the CIFAR training example from the official GitHub repository.