Raise NotImplementedError at conversion time for reflect/replicate pad >2D (fixes #2576, #2571)#2701
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
…d >2D
`mb.pad` only supports `reflect` / `replicate` modes when at most the final
two dimensions carry non-zero padding. The generic `pad` converter
forwarded the mode through unconditionally, which produced a confusing
model-compile-time failure inside `predict()`:
Failed to parse the model specification. Error: Unable to parse ML
Program: in operation pad_cast_fp16: Padding for more than two
dimensions only supports `constant` mode
This patch checks the (static) padding profile inside `_translate_torch_args`
and raises a clear `NotImplementedError` at conversion time instead. The
check fires for the torch.export-decomposed shape of `ReflectionPad3d` /
`ReplicationPad3d` with non-trivial padding (`aten.pad` with mode=reflect
or replicate and >2 dims of non-zero pad).
A regression test under `TestPad` covers both torch modules.
Fixes apple#2576
Fixes apple#2571
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mb.padonly supportsreflect/replicatemodes when at most the final two dimensions carry non-zero padding. The genericpadtorch converter forwards the mode through unconditionally, so torch.export-decomposedtorch.nn.ReflectionPad3d/torch.nn.ReplicationPad3d(which decompose toaten.padwithmode="reflect"/"replicate"and 3 padded dims) currently convert successfully and then explode atpredict()time with:The two original issues (#2576 ReflectionPad3D, #2571 ReplicationPad3d) reported this exact failure mode and asked for an ahead-of-time error instead.
Fix
Added a static check inside
_translate_torch_argsin thepadop converter (coremltools/converters/mil/frontend/torch/ops.py). Whenmode in ("reflect", "replicate"), thepadlist is fully static, and more than two dimensions have non-zero pad, raiseNotImplementedErrorwith a message that names the mode, the padded-dim count, and the typical culprit modules.The check is scoped tightly:
pad(dynamic-padding path was already raising for other reasons);reflect/replicatemodes (constant and circular unaffected);padded_dims > 2— valid 2D cases (ReflectionPad2d, etc.) still convert unchanged.Tests
Added
TestPad::test_pad_reflect_replicate_3d_raisesparameterized over bothtorch.nn.ReflectionPad3dandtorch.nn.ReplicationPad3d:I also manually verified
torch.nn.ReflectionPad2d(padding=2)on a rank-4 input still passes the new guard and proceeds to the rest of the converter.Issues
Fixes #2576
Fixes #2571