[CoreML MLProgram] Support Float16 (1/N) #22068
Conversation
wejoncy
force-pushed
the
jicwen/coreml_fp16
branch
from
e134133
to
305e9ab
Compare
| @@ -114,6 +117,11 @@ void CreateCoreMLWeight(CoreML::Specification::WeightParams& weight, gsl::span<c | |||
| weight.mutable_floatvalue()->Assign(data.begin(), data.end()); | |||
| } | |||
|
|
|||
| void CreateCoreMLWeight(CoreML::Specification::WeightParams& weight, gsl::span<const MLFloat16> data) { | |||
There was a problem hiding this comment.
Do we need CreateCoreMLWeightConvertingDataToFloats to also support converting to fp16? Maybe not until we add fp16 support to operators that have initializers as input.
There was a problem hiding this comment.
Already support CreateCoreMLWeightConvertingDataToFloat16s
| template MILSpec::Value CreateTensorValue<float, float>(gsl::span<const float> data, | ||
| std::optional<gsl::span<const int64_t>> shape); | ||
| template MILSpec::Value CreateTensorValue<bool, bool>(gsl::span<const bool> data, | ||
| std::optional<gsl::span<const int64_t>> shape); | ||
| template MILSpec::Value CreateTensorValue<std::string, std::string>(gsl::span<const std::string> data, | ||
| std::optional<gsl::span<const int64_t>> shape); |
There was a problem hiding this comment.
Why do we need these new specializations to handle fp16 data?
There was a problem hiding this comment.
The Linux-GCC compiler complains that CreateTensorValue<float, float> is not defined. The template instantiation helps to suppression it.
There was a problem hiding this comment.
Does that mean we use these new combinations in the EP implementation somewhere and the change is just making everything consistent?
There was a problem hiding this comment.
Yes, It's a forward declaration for specific function signature.
| @@ -257,6 +257,98 @@ TEST(CoreMLExecutionProviderTest, TestNameSanitization) { | |||
| // TensorRT does not support Clip opset 11 yet. | |||
| test.Run(OpTester::ExpectResult::kExpectSuccess, "", {kTensorrtExecutionProvider}); | |||
| } | |||
|
|
|||
| TEST(CoreMLExecutionProviderTest, TestBinaryFp16) { | |||
There was a problem hiding this comment.
Do we need to add separate tests here or could we update onnxruntime\test\providers\cpu\math\element_wise_ops_test.cc to run the MLFloat16 tests it has for CoreML?
We're also going to add some xnnpack fp16 kernels so the more common test code that is used the better.
There was a problem hiding this comment.
Good idea. Refactored some of elemenwise UTs. It's still a bit messy for FP16/Bf16 test.
wejoncy
force-pushed
the
jicwen/coreml_fp16
branch
from
305e9ab
to
3944fd6
Compare
wejoncy
force-pushed
the
jicwen/coreml_fp16
branch
from
957c4d9
to
4f935e7
Compare
Description
Support Float16 for CoreML MLProgram EP.
Operations:
Unary/Binary/Act
Motivation and Context