[CoreML EP] Add HardSigmoid support#28182
Conversation
|
@microsoft-github-policy-service agree |
d2e815c to
72940ee
Compare
|
Amended the branch to add a dedicated CoreML-EP test and correct the earlier claim about test coverage. My original PR description asserted that the existing The new test in This pattern worth keeping in mind more broadly — the recent Softplus/Elu addition (#26462) also relies on the multi-EP CPU test and may not be catching CoreML-side regressions either. |
There was a problem hiding this comment.
Pull request overview
Adds HardSigmoid operator coverage to the CoreML Execution Provider so models using this activation no longer fall back to CPU (avoiding CoreML↔CPU graph breaks) while maintaining output parity with the CPU reference.
Changes:
- Implement
HardSigmoidin CoreML EP activation builder for both MLProgram (sigmoid_hard) and NeuralNetwork (ActivationSigmoidHard) paths, includingalpha/betawiring. - Register
HardSigmoidin the CoreML op builder factory. - Add a dedicated CoreML EP test that verifies full-node assignment and output correctness in both NN and MLProgram formats; update the supported-ops doc list.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tools/ci_build/github/apple/coreml_supported_mlprogram_ops.md | Documents ai.onnx:HardSigmoid as supported for MLProgram. |
| onnxruntime/test/providers/coreml/coreml_basic_test.cc | Adds a single-node HardSigmoid model test verifying full CoreML assignment and correct outputs (NN + MLProgram). |
| onnxruntime/core/providers/coreml/builders/op_builder_factory.cc | Registers HardSigmoid with the activation op builder. |
| onnxruntime/core/providers/coreml/builders/impl/activation_op_builder.cc | Implements HardSigmoid conversion for MLProgram and NeuralNetwork model formats and lists it as a supported activation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The PR may require rebase from main when the pipelines are fixed. |
Adds `HardSigmoid` to the CoreML Execution Provider's activation op builder. Both MLProgram (`sigmoid_hard`) and NeuralNetwork (`ActivationSigmoidHard`) code paths are implemented; the op's ONNX definition matches CoreML MIL's `sigmoid_hard` exactly, so no decomposition is required. Adds a dedicated CoreML-EP test `CoreMLExecutionProviderTest.HardSigmoidTest` that verifies the entire graph is placed on the CoreML EP (both NN and MLProgram formats) via `ExpectedEPNodeAssignment::All`, and that the output matches the CPU reference. The existing multi-EP test in `activation_op_test.cc` silently falls back to CPU for unsupported-on-EP ops, so a dedicated test is required to genuinely verify the CoreML path. Also updates coreml_supported_mlprogram_ops.md. Fixes microsoft#28181. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
72940ee to
81c4421
Compare
|
Force push makes it hard to review the changes. |
|
Apologies — I force-pushed after rebasing on main and amending. Won't repeat that pattern; I'll stack follow-up commits instead so the review-since-last diff stays usable. For this round, what changed on top of the original commit (
Range-diff if helpful: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/azp run Win_TRT_Minimal_CUDA_Test_CI, Windows GPU Doc Gen CI Pipeline |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Description
Adds
HardSigmoidto the CoreML Execution Provider's activation op builder. Both MLProgram (sigmoid_hard) and NeuralNetwork (ActivationSigmoidHard) code paths are implemented; the op's ONNX definition matches CoreML MIL'ssigmoid_hardexactly, so no decomposition is required.Adds a dedicated CoreML-EP test (
CoreMLExecutionProviderTest.HardSigmoidTest) that builds a single-node HardSigmoid model with non-defaultalpha/betaand usesRunAndVerifyOutputsWithEPwithExpectedEPNodeAssignment::Allto confirm (a) the entire graph is claimed by the CoreML EP in both NN and MLProgram formats, and (b) the output matches the CPU reference. I verified the test is not trivially passing by temporarily unregistering HardSigmoid from the activation builder — the test fails withVerifyEPNodeAssignmentemitting a fatal failure, proving it genuinely exercises the CoreML path. (The existing multi-EP test inactivation_op_test.ccsilently falls back to CPU when an EP rejects the node, so it does not give CoreML coverage on its own.)Also updates
coreml_supported_mlprogram_ops.md.Motivation and Context
Fixes #28181.
On a DWPose pose-estimation model (
dw-ll_ucoco_384.onnx), 4 HardSigmoid ops were each forcing a CoreML → CPU → CoreML round-trip, and also caused downstream ops to be rejected with "unsupported inputs" because their producers had been sent to CPU. Adding HardSigmoid collapses the graph from 5 CoreML subgraphs to 1, and drops inference from 9.22 ms to 6.92 ms (−25%) on Apple Silicon with MLProgram + ComputeUnits=ALL.