Skip to content

[java] Fp16 fix for android/react native#16832

Merged
YUNQIUGUO merged 5 commits intomicrosoft:mainfrom
Craigacp:android-fp16-fix
Jul 25, 2023
Merged

[java] Fp16 fix for android/react native#16832
YUNQIUGUO merged 5 commits intomicrosoft:mainfrom
Craigacp:android-fp16-fix

Conversation

@Craigacp
Copy link
Contributor

Description

This PR splits out the FP16 conversions into a separate package we can override in the android build with a version which works on old versions of Android.

I'm not sure the android build system changes are correct as I haven't got an android build environment configured on my workstation. @YUNQIUGUO if the CI build fails we should follow up offline to get my environment configured so I can iterate on it.

Motivation and Context

Fixes the CI failure after #16703.

@yuslepukhin
Copy link
Member

/azp run MacOS CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-python-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, Windows ARM64 QNN CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16832 in repo microsoft/onnxruntime

@yuslepukhin
Copy link
Member

/azp run orttraining-linux-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run orttraining-linux-gpu-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run orttraining-ortmodule-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@YUNQIUGUO
Copy link
Contributor

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Craigacp
Copy link
Contributor Author

One day I'll remember to run spotless after making small changes before making the commit, but it wasn't today apparently. I've pushed the spotless fixes so those test failures out of the Java side should go away and we'll hit the actual code changes.

@Craigacp
Copy link
Contributor Author

Looks like Android wasn't happy about the packaging changes either, so I'll need to iterate on that (https://dev.azure.com/onnxruntime/onnxruntime/_build/results?buildId=1074203&view=logs&j=06bda0f0-c359-5ee8-d76d-283c2733373e&t=af31bba3-008c-59d3-c2d1-f81f08a354ff&l=6566).

@yuslepukhin
Copy link
Member

/azp run MacOS CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-python-checks-ci-pipeline

@yuslepukhin
Copy link
Member

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, Windows ARM64 QNN CI Pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16832 in repo microsoft/onnxruntime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run ONNX Runtime React Native CI Pipeline

@yuslepukhin
Copy link
Member

/azp run orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, Windows ARM64 QNN CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@yuslepukhin
Copy link
Member

/azp run Windows_SCA

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@yuslepukhin
Copy link
Member

/azp run Windows_SCA/Onnxruntime-SCA-training-CUDA

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@yuslepukhin
Copy link
Member

/azp run MacOS CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-python-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@YUNQIUGUO
Copy link
Contributor

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Craigacp
Copy link
Contributor Author

Craigacp commented Jul 25, 2023

After further consideration I split the Java code into three source roots. The shared code is in src/main/java, the Android specific stuff (which at the moment is just the FP16 conversions) is in src/main/android and the Java/JVM specific stuff is in src/main/jvm. I've tested the Java build and that works, and the Android build seems to be failing at a different point further on than the React pipeline error that we saw which I think is due to the misconfigured Android build environment I've cobbled together to test it. I'm hopeful that this will work, but it's probably worth running just the react native pipeline and one of the other ones with Java turned on first to make it easier for me to see the failures (which hopefully there won't be).

@YUNQIUGUO
Copy link
Contributor

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mszhanyi
Copy link
Contributor

/azp run Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 10 pipeline(s).

@microsoft microsoft deleted a comment from azure-pipelines bot Jul 25, 2023
@microsoft microsoft deleted a comment from azure-pipelines bot Jul 25, 2023
@mszhanyi
Copy link
Contributor

/azp run Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline

@microsoft microsoft deleted a comment from azure-pipelines bot Jul 25, 2023
@azure-pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

Copy link
Contributor

@YUNQIUGUO YUNQIUGUO left a comment

Choose a reason for hiding this comment

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

thanks for the fix!

@YUNQIUGUO YUNQIUGUO merged commit a1bb670 into microsoft:main Jul 25, 2023
jchen351 pushed a commit that referenced this pull request Aug 12, 2023
### Description
This PR splits out the FP16 conversions into a separate package we can
override in the android build with a version which works on old versions
of Android.

I'm not sure the android build system changes are correct as I haven't
got an android build environment configured on my workstation.
@YUNQIUGUO if the CI build fails we should follow up offline to get my
environment configured so I can iterate on it.

### Motivation and Context
Fixes the CI failure after #16703.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants