Skip to content
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

Add UINT8 datatype support to Java #8401

Merged
merged 2 commits into from
Jul 23, 2021
Merged

Conversation

frankfliu
Copy link
Contributor

Current onnexruntime Java doesn't support uint8 datatype. See: #6261

The native part already have uint8 support, the memory in the native side actually are the same between uint8 and int8. We can use signed byte[] to create uint8 Tensor.

The uint8 on java side can be presented as byte[] and end user can manually convert it to unsigned if needed.

With this change user can run uint8 model without hacking the model itself.

@frankfliu frankfliu requested a review from a team as a code owner July 15, 2021 02:26
@ghost
Copy link

ghost commented Jul 15, 2021

CLA assistant check
All CLA requirements met.

@snnn
Copy link
Member

snnn commented Jul 15, 2021

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline

@snnn
Copy link
Member

snnn commented Jul 15, 2021

/azp run MacOS NoContribops CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

Copy link
Contributor

@Craigacp Craigacp left a comment

Choose a reason for hiding this comment

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

Looks fine, though could you add a test along the lines of https://github.com/microsoft/onnxruntime/blob/master/java/src/test/java/ai/onnxruntime/InferenceTest.java#L1206 to make sure that it can go into and out of a model correctly. There is a test_types_UINT8.pb so it should be straightforward to do.

@frankfliu
Copy link
Contributor Author

Looks fine, though could you add a test along the lines of https://github.com/microsoft/onnxruntime/blob/master/java/src/test/java/ai/onnxruntime/InferenceTest.java#L1206 to make sure that it can go into and out of a model correctly. There is a test_types_UINT8.pb so it should be straightforward to do.

I added model unit test. But the CI seems stuck.

@Craigacp
Copy link
Contributor

Someone from MS needs to retrigger it as you added commits.

@fdwr fdwr changed the title Add UINT8 datatype support Add UINT8 datatype support to Java Jul 18, 2021
@yuslepukhin
Copy link
Member

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline

@yuslepukhin
Copy link
Member

/azp run MacOS NoContribops CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@yuslepukhin
Copy link
Member

MacOS pipelines failing to start. Will follow up.

@yuslepukhin
Copy link
Member

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline

@yuslepukhin
Copy link
Member

/azp run MacOS NoContribops CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

Copy link
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

Looks good

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.

4 participants