Fix incorrect static shape in depthwise_conv and separable_conv when strides > 1 and dilation_rate > 1#22598
Conversation
…le_conv ops The low-level keras.ops.depthwise_conv and keras.ops.separable_conv functions lacked the same strides > 1 + dilation_rate > 1 validation that already exists in the high-level DepthwiseConv2D and SeparableConv2D layers. This caused the symbolic (static) shape inference to return incorrect shapes when both strides > 1 and dilation_rate > 1 are used together, since the formula-based shape computation disagrees with what TF's depthwise/separable conv ops actually produce. Also added the validation to both ops and updated the tests to assert a ValueError instead of silently skipping the unsupported combination on the TF backend.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces validation in depthwise_conv and separable_conv to prevent the simultaneous use of strides and dilation rates greater than one, which is not supported. The implementation raises a ValueError when these conditions are met, and the tests have been updated to assert this behavior. Feedback focuses on improving maintainability by extracting the duplicated max-value calculation logic into a helper function and ensuring that the type-checking in tests is consistent with the implementation by supporting both lists and tuples.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #22598 +/- ##
=======================================
Coverage 83.28% 83.29%
=======================================
Files 596 596
Lines 68089 68095 +6
Branches 10607 10609 +2
=======================================
+ Hits 56711 56717 +6
Misses 8634 8634
Partials 2744 2744
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…s+dilation The static and dynamic shape tests were asserting on output shapes for strides=2 + dilation_rate=2 combinations, which are now correctly rejected with a ValueError. Also extract _get_seq_max helper to avoid duplicated logic in depthwise_conv and separable_conv, and use (list, tuple) consistently in the test isinstance checks.
3 participants
The low-level
keras.ops.depthwise_convandkeras.ops.separable_convops were missing a validation that already exists in the high-levelDepthwiseConv2DandSeparableConv2Dlayers: usingstrides > 1together withdilation_rate > 1is not supported. Without it, the static shape returned during tracing was computed using the standard convolution formula, which disagrees with what the underlying TF ops actually produce, leading to a silent shape mismatch. The fix raises a clearValueErrorat call time (both eager and symbolic) and updates the tests to assert on the error instead of silently skipping the case on the TF backend.