Slice over dynamic dimension#5015
Open
CharlieL7 wants to merge 8 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates MIGraphX’s slice op shape inference to support slicing over non-fixed dynamic dimensions (notably needed for post-NMS workloads), by relaxing sliced axes to {0, interval.max} for the 1-input/attribute-only slice form, and it expands the shape-test coverage accordingly.
Changes:
- Allow 1-arg
sliceon non-fixed dynamic (range) axes by returning an over-approximated output dynamic dimension{0, max}. - Keep rejecting the same case for symbolic shapes (explicit throw).
- Update and add
slicedynamic-shape tests to reflect and validate the new behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/op_shape_test.cpp | Updates expectations for non-fixed dynamic slicing and adds several new shape tests around the relax behavior. |
| src/include/migraphx/op/slice.hpp | Changes 1-arg slice shape inference to relax sliced non-fixed dynamic axes (and throws for symbolic). |
Comment on lines
276
to
+280
| if(input_shape.dynamic() and std::any_of(axes.begin(), axes.end(), [&](auto axis) { | ||
| return not input_shape.dyn_dims()[axis].is_fixed(); | ||
| })) | ||
| { | ||
| MIGRAPHX_THROW("SLICE 1_arg: slicing is not allowed on non-fixed dynamic input axis "); | ||
| if(input_shape.symbolic()) |
Collaborator
Author
There was a problem hiding this comment.
As far as I can tell, there is no at() for dynamic_dimensions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5015 +/- ##
===========================================
+ Coverage 92.71% 92.72% +0.01%
===========================================
Files 594 596 +2
Lines 31482 31739 +257
===========================================
+ Hits 29187 29427 +240
- Misses 2295 2312 +17
🚀 New features to boost your workflow:
|
Regressions detected 🔴 * No develop baseline was found for this PR's branch point; compared against the latest available develop run instead. |
|
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
shivadbhavsar
approved these changes
Jun 30, 2026
kahmed10
reviewed
Jul 1, 2026
| auto dds = input_shape.dyn_dims(); | ||
| for(auto axis : this->axes) | ||
| { | ||
| dds[axis] = {0, dds[axis].get_interval().max}; |
Collaborator
There was a problem hiding this comment.
do we need to use 0 here for the min, or can we use 1?
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.
Motivation
Technical Details
{0, interval.max}Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot Applicable