Remove asserts in CoreML resize#17626
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17626
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 1 Unrelated FailureAs of commit 404f1d5 with merge base 4dadf24 ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@metascroy has exported this pull request. If you are a Meta employee, you can view the originating Diff in D93942192. |
This PR needs a
|
1aefccd to
a616479
Compare
Summary: The `assert(shape[i] <= shape_[i])` constraint in `MultiArray::MemoryLayout::resize()` was unnecessary and overly restrictive. `MemoryLayout` is purely metadata describing shape and strides - it has no knowledge of the underlying memory allocation. The actual memory safety is guaranteed at a higher level: ExecuTorch tensors are pre-allocated with sufficient capacity to handle dynamic output shapes from CoreML model execution. The resize function is called in `ETCoreMLModelManager.mm` to update output tensor metadata after model inference, where the new shape comes directly from CoreML's `modelOutputs[i].shape`. Since the ExecuTorch output buffers are sized appropriately for the model's maximum output dimensions, the resize operation is always safe regardless of whether the new shape is smaller or larger than the previous metadata indicated. The remaining asserts are retained because they catch actual programming errors: - `assert(shape.size() == shape_.size())` - prevents rank mismatch causing out-of-bounds access - `assert(shape[i] >= 1)` - ensures valid dimensions for correct stride computation Reviewed By: shabbyowen, GregoryComer Differential Revision: D93942192
a616479 to
7ee7013
Compare
Summary: The `assert(shape[i] <= shape_[i])` constraint in `MultiArray::MemoryLayout::resize()` was unnecessary and overly restrictive. `MemoryLayout` is purely metadata describing shape and strides - it has no knowledge of the underlying memory allocation. The actual memory safety is guaranteed at a higher level: ExecuTorch tensors are pre-allocated with sufficient capacity to handle dynamic output shapes from CoreML model execution. The resize function is called in `ETCoreMLModelManager.mm` to update output tensor metadata after model inference, where the new shape comes directly from CoreML's `modelOutputs[i].shape`. Since the ExecuTorch output buffers are sized appropriately for the model's maximum output dimensions, the resize operation is always safe regardless of whether the new shape is smaller or larger than the previous metadata indicated. The remaining asserts are retained because they catch actual programming errors: - `assert(shape.size() == shape_.size())` - prevents rank mismatch causing out-of-bounds access - `assert(shape[i] >= 1)` - ensures valid dimensions for correct stride computation Reviewed By: shabbyowen, GregoryComer Differential Revision: D93942192
Summary: The `assert(shape[i] <= shape_[i])` constraint in `MultiArray::MemoryLayout::resize()` was unnecessary and overly restrictive. `MemoryLayout` is purely metadata describing shape and strides - it has no knowledge of the underlying memory allocation. The actual memory safety is guaranteed at a higher level: ExecuTorch tensors are pre-allocated with sufficient capacity to handle dynamic output shapes from CoreML model execution. The resize function is called in `ETCoreMLModelManager.mm` to update output tensor metadata after model inference, where the new shape comes directly from CoreML's `modelOutputs[i].shape`. Since the ExecuTorch output buffers are sized appropriately for the model's maximum output dimensions, the resize operation is always safe regardless of whether the new shape is smaller or larger than the previous metadata indicated. The remaining asserts are retained because they catch actual programming errors: - `assert(shape.size() == shape_.size())` - prevents rank mismatch causing out-of-bounds access - `assert(shape[i] >= 1)` - ensures valid dimensions for correct stride computation Reviewed By: shabbyowen, GregoryComer Differential Revision: D93942192
7ee7013 to
976bfba
Compare
Summary: The `assert(shape[i] <= shape_[i])` constraint in `MultiArray::MemoryLayout::resize()` was unnecessary and overly restrictive. `MemoryLayout` is purely metadata describing shape and strides - it has no knowledge of the underlying memory allocation. The actual memory safety is guaranteed at a higher level: ExecuTorch tensors are pre-allocated with sufficient capacity to handle dynamic output shapes from CoreML model execution. The resize function is called in `ETCoreMLModelManager.mm` to update output tensor metadata after model inference, where the new shape comes directly from CoreML's `modelOutputs[i].shape`. Since the ExecuTorch output buffers are sized appropriately for the model's maximum output dimensions, the resize operation is always safe regardless of whether the new shape is smaller or larger than the previous metadata indicated. The remaining asserts are retained because they catch actual programming errors: - `assert(shape.size() == shape_.size())` - prevents rank mismatch causing out-of-bounds access - `assert(shape[i] >= 1)` - ensures valid dimensions for correct stride computation Reviewed By: shabbyowen, GregoryComer Differential Revision: D93942192
976bfba to
88c0482
Compare
Summary: Pull Request resolved: pytorch#17626 The `assert(shape[i] <= shape_[i])` constraint in `MultiArray::MemoryLayout::resize()` was unnecessary and overly restrictive. `MemoryLayout` is purely metadata describing shape and strides - it has no knowledge of the underlying memory allocation. The actual memory safety is guaranteed at a higher level: ExecuTorch tensors are pre-allocated with sufficient capacity to handle dynamic output shapes from CoreML model execution. The resize function is called in `ETCoreMLModelManager.mm` to update output tensor metadata after model inference, where the new shape comes directly from CoreML's `modelOutputs[i].shape`. Since the ExecuTorch output buffers are sized appropriately for the model's maximum output dimensions, the resize operation is always safe regardless of whether the new shape is smaller or larger than the previous metadata indicated. The remaining asserts are retained because they catch actual programming errors: - `assert(shape.size() == shape_.size())` - prevents rank mismatch causing out-of-bounds access - `assert(shape[i] >= 1)` - ensures valid dimensions for correct stride computation Reviewed By: shabbyowen, GregoryComer Differential Revision: D93942192
88c0482 to
404f1d5
Compare
Summary:
The
assert(shape[i] <= shape_[i])constraint inMultiArray::MemoryLayout::resize()was unnecessary and overly restrictive.MemoryLayoutis purely metadata describing shape and strides - it has no knowledge of the underlying memory allocation. The actual memory safety is guaranteed at a higher level: ExecuTorch tensors are pre-allocated with sufficient capacity to handle dynamic output shapes from CoreML model execution.The resize function is called in
ETCoreMLModelManager.mmto update output tensor metadata after model inference, where the new shape comes directly from CoreML'smodelOutputs[i].shape. Since the ExecuTorch output buffers are sized appropriately for the model's maximum output dimensions, the resize operation is always safe regardless of whether the new shape is smaller or larger than the previous metadata indicated.The remaining asserts are retained because they catch actual programming errors:
assert(shape.size() == shape_.size())- prevents rank mismatch causing out-of-bounds accessassert(shape[i] >= 1)- ensures valid dimensions for correct stride computationReviewed By: GregoryComer
Differential Revision: D93942192