[CoreML] Adapt to MLMultiArray.dataPointer being deprecated
#17652
Comments
NickLucche
added
the
platform:mobile
|
Good point. It's probably a good idea to move away from the deprecated API in case it gets removed later. I think maybe we can use getBytesWithHandler: and verify that we have a contiguous layout by checking strides. |
|
Sounds like a plan, if it's not contiguous we just copy each block into |
|
Sorry for the delayed response. I think either approach is fine. It may be simpler to just treat a non-contiguous buffer as unsupported initially and we can see if we actually do run into such a case. PRs are welcome. :) Otherwise, we can get to it eventually. |
|
Sure I am on it :) |
|
This issue has been automatically marked as stale due to inactivity and will be closed in 7 days if no further activity occurs. If further support is needed, please provide an update and/or more details. |
github-actions
bot
added
the
stale
|
Update: PR #17726 is open, waiting for input about minimum MacOS version to support. |
edgchen1
removed
the
stale
### Description This PR addresses #17652. The deprecated `MLMultiArray.dataPointer` is replaced with `.getBytesWithHandler`, as suggested by the docs. For now, I am only checking that the output `MLMultiArray` is contiguous, returning unsupported operation when that is not the case. I think this is already better than what we have right now, so we can block unsafe calls to `.dataPointer` (if any..). I would be happy to implement the handling of the non-contiguous case (replacing `memcpy` for such cases) as suggested by @edgchen1, but I am not sure how to reproduce that case to add a corresponding unit-test. Would we have to define a custom `MLCustomLayer` to get a non-contiguous output from a model..? ### Motivation and Context Fix #17652. --------- Co-authored-by: nicolo-lucchesi <[email protected]> Co-authored-by: Edward Chen <[email protected]>
…7726) ### Description This PR addresses microsoft#17652. The deprecated `MLMultiArray.dataPointer` is replaced with `.getBytesWithHandler`, as suggested by the docs. For now, I am only checking that the output `MLMultiArray` is contiguous, returning unsupported operation when that is not the case. I think this is already better than what we have right now, so we can block unsafe calls to `.dataPointer` (if any..). I would be happy to implement the handling of the non-contiguous case (replacing `memcpy` for such cases) as suggested by @edgchen1, but I am not sure how to reproduce that case to add a corresponding unit-test. Would we have to define a custom `MLCustomLayer` to get a non-contiguous output from a model..? ### Motivation and Context Fix microsoft#17652. --------- Co-authored-by: nicolo-lucchesi <[email protected]> Co-authored-by: Edward Chen <[email protected]>
Describe the issue
Hey,
I've seen we're using this
MLMultiArray.dataPointermethod here https://github.com/microsoft/onnxruntime/blob/main/onnxruntime/core/providers/coreml/model/model.mm#L327 to access the underlying memory before moving that to a C lifetime-managed location.I noticed this method appears to be deprecated on Apple docs. I am merely speculating, but maybe is it because they support non-contiguously stored
MLMultiArrays..?My question is should we do something about this, like using some of the other non-deprecated element access methods (although it may decrease performance compared to
memcpy), or are we sure this won't ever cause issues going forward?To reproduce
Urgency
No response
Platform
iOS
OS Version
/
ONNX Runtime Installation
Released Package
Compiler Version (if 'Built from Source')
No response
Package Name (if 'Released Package')
None
ONNX Runtime Version or Commit ID
1.16.0
ONNX Runtime API
C++/C
Architecture
ARM64
Execution Provider
CoreML
Execution Provider Library Version
No response
The text was updated successfully, but these errors were encountered: