Android input and output tag JNI #11057
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11057
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 06f8e7b with merge base df5e7df ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
kirklandsign
added
the
release notes: api
There was a problem hiding this comment.
Pull Request Overview
This PR introduces new JNI helper methods to retrieve input and output tags for Android, and updates the corresponding Java API and tests.
- Added getInputTags and getOutputTags methods in the JNI layer.
- Updated the Java Module and MethodMetadata classes to store and use the new tag information.
- Adjusted tests to verify the new behavior.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| extension/android/jni/jni_layer.cpp | Added native methods getInputTags and getOutputTags and registered them; minor variable renaming adjustments. |
| extension/android/executorch_android/src/main/java/org/pytorch/executorch/Module.java | Updated to declare native methods for input and output tags and populate MethodMetadata with them. |
| extension/android/executorch_android/src/main/java/org/pytorch/executorch/MethodMetadata.java | Extended the class to include inputTags and outputTags with corresponding getters and setters. |
| extension/android/executorch_android/src/androidTest/java/org/pytorch/executorch/ModuleInstrumentationTest.kt | Added assertions to check the returned input and output tags. |
| extension/android/executorch_android/src/androidTest/java/org/pytorch/executorch/ModuleE2ETest.kt | Updated expected backend array formatting. |
extension/android/jni/jni_layer.cpp
Outdated
| facebook::jni::local_ref<facebook::jni::JArrayInt> ret = | ||
| facebook::jni::JArrayInt::newArray(num_inputs); | ||
|
|
||
| int i = 0; |
There was a problem hiding this comment.
The outer declaration of the variable 'i' is redundant as the for-loop declares its own 'i'. Remove the unnecessary declaration to improve clarity.
| int i = 0; |
Copilot uses AI. Check for mistakes.
| facebook::jni::JArrayInt::newArray(num_outputs); | ||
|
|
||
| int i = 0; | ||
| for (int i = 0; i < num_outputs; i++) { |
There was a problem hiding this comment.
The variable 'i' is declared before the for-loop and then re-declared in the loop, which shadows the outer variable. Removing the redundant declaration will prevent potential confusion.
| for (int i = 0; i < num_outputs; i++) { | |
| for (i = 0; i < num_outputs; i++) { |
Copilot uses AI. Check for mistakes.
kirklandsign
force-pushed
the
android-metadata-2
branch
from
8a2704f to
0d38a99
Compare
|
@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary
Add JNI helper to get tags. Add API later.
Test plan
CI