fix: support Float16/BFloat16/Float8 in TensorArray custom op#28335
Conversation
The TensorArray (Variadic) constructor's switch in onnxruntime_lite_custom_op.h was missing case arms for Float16, BFloat16, and the four Float8 variants, so any custom op accepting const Ort::Custom::TensorArray& with those input types threw "unknown input type" at construction. Add the six missing case arms, mirroring the type-name parity already established in CREATE_TUPLE and PARSE_ARGS within the same header. Fixes microsoft#23373
There was a problem hiding this comment.
Pull request overview
Fixes Lite Custom Op API support for variadic tensor inputs (Ort::Custom::TensorArray / Variadic) by adding missing element-type dispatch cases so fp16/bf16/fp8 inputs no longer fall through to "unknown input type".
Changes:
- Add
ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT16andONNX_TENSOR_ELEMENT_DATA_TYPE_BFLOAT16handling inTensorArrayconstruction. - Add handling for the four float8 element types (
FLOAT8E4M3FN,FLOAT8E4M3FNUZ,FLOAT8E5M2,FLOAT8E5M2FNUZ) to match existing support elsewhere in the header.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The only failing check is All other lanes are green: Linux/Windows/Android/Mac/WASM/WebGPU/CUDA/TensorRT/QNN/CoreML/iOS-on-Mac builds, plus Could a maintainer familiar with the Lite Custom Op / |
|
Thanks for the review summary. To confirm intent: the Float16/BFloat16 cases close the original gap that motivated this PR, and the four Float8 variants (E4M3FN, E4M3FNUZ, E5M2, E5M2FNUZ) were added in the same dispatch table for symmetry with the rest of the header, which already exposes those dtypes via |
tianleiwu
left a comment
There was a problem hiding this comment.
Review Summary
Small, correct, well-scoped fix. The six new case arms (FLOAT16, BFLOAT16, and the four FLOAT8* variants) added to the TensorArray input-construction switch match the template arguments and enum→type mapping already used by CREATE_TUPLE (lines ~619–637) and PARSE_ARGS (lines ~744–762) in the same header. The change is header-only, purely additive, with no API/ABI impact and no behavior change for previously supported types. The Float8* types are referenced unconditionally elsewhere in the header, so no new conditional-compilation requirement is introduced. Verified parity — looks good.
Suggestion: add a regression test
The PR adds no test exercising the new types. Existing variadic coverage (CopyTensorArrayAllVariadic, CopyTensorArrayCombined in onnxruntime/test/testdata/custom_op_library/cpu/cpu_ops.cc, driven from onnxruntime/test/shared_lib/test_inference.cc) is float-only. Per the contribution guidance, a behavioral change like this should be covered by a test. A minimal const TensorArray& custom op run with an fp16 (and/or bf16) input would lock in the fix and guard this switch against future regressions. fp8 is harder to construct end-to-end, so fp16 is the highest-value addition.
Pre-existing observation (out of scope for this PR)
In the same constructor loop, the element type is read from ctx_.GetInput(start_indice) instead of ctx_.GetInput(ith_input):
for (size_t ith_input = start_indice; ith_input < input_count; ++ith_input) {
auto const_value = ctx_.GetInput(start_indice); // always the first input
auto type = const_value.GetTensorTypeAndShapeInfo().GetElementType();
...
tensor = std::make_unique<Custom::Tensor<T>>(ctx, ith_input, true);The constructed Custom::Tensor<T> correctly uses ith_input, but the type used to select T is always taken from the first input. For homogeneous variadic inputs (the common case) this is harmless; for a heterogeneous input list each tensor would be constructed with the first input's type. This is pre-existing and not introduced by this PR, but since the PR touches exactly this switch, a follow-up changing it to GetInput(ith_input) would be worthwhile.
Description
The
Ort::Custom::TensorArray(aliasVariadic) constructor ininclude/onnxruntime/core/session/onnxruntime_lite_custom_op.hwas missingcasearms forFloat16,BFloat16, and the fourFloat8*variants. Any custom op acceptingconst Ort::Custom::TensorArray&with one of those input types fell through to thedefault:arm and threw"unknown input type"at construction.This PR adds the six missing
casearms so the constructor handles every floating-point variant the rest of the file already supports. The new arms mirror the type-name parity already established inCREATE_TUPLE(around lines 619-637) andPARSE_ARGS(around lines 744-762) within the same header — sameOrt::Float16_t/Ort::BFloat16_t/Ort::Float8E4M3FN_t/Ort::Float8E4M3FNUZ_t/Ort::Float8E5M2_t/Ort::Float8E5M2FNUZ_ttemplate arguments.The change is purely additive: 18 lines, header-only, no API/ABI impact, no behavior change for any previously-supported type.
Motivation and Context
Fixes #23373.
Custom ops authored against the Lite Custom Op API and declared with a
const Ort::Custom::TensorArray¶meter currently cannot accept fp16, bf16, or fp8 inputs even thoughCustom::Tensor<Ort::Float16_t>etc. are first-class everywhere else in the API. This blocks half-precision and 8-bit-float variadic custom op authors on CPU EP and any EP that dispatches via the lite API.