Skip to content

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

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

IshanAryendu
Copy link

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.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 15, 2025
@psiddh psiddh self-requested a review July 15, 2025 16:46
@jathu
Copy link

jathu commented Jul 15, 2025

@IshanAryendu I think we should follow the current pattern of the repo and put this under cifar/android instead of cifar/app

cifar/README.md Outdated

For the android environment setup, follow these steps:

1. Install Open JDK using: `brew install openjdk@17`
Copy link

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)

Copy link
Author

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/
Copy link

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}")

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?

Copy link
Author

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}")

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.

Copy link
Author

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")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inconsistent tag

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the inconsistency

Copy link

@larryliu0820 larryliu0820 left a 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
Copy link
Contributor

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

Copy link
Author

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

Comment on lines 44 to 51
```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
```
Copy link

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?

Copy link
Author

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

Copy link

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?

Copy link
Author

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

Copy link

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

@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like Android standard

Copy link

@larryliu0820 larryliu0820 Jul 18, 2025

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?

Comment on lines 44 to 51
```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
```
Copy link

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

@larryliu0820
Copy link

Please address @jathu's comments


## Creation of Android App

1. Start with a clone of this repository and open the project in Android Studio.
Copy link
Contributor

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 ?

Copy link
Author

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?

@IshanAryendu IshanAryendu requested a review from psiddh July 18, 2025 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants