Fix slice op issues #13760#14403
Conversation
|
This PR addresses the comments posted on an earlier PR #14107 (closed). |
|
Please review @reminisce @szha @anirudh2290 |
|
@mxnet-label-bot add [Operator, pr-awaiting-review] |
There was a problem hiding this comment.
There are so many if/else/elseif branches in this piece of code. I know it fixes the two cases you mentioned in the description. However, instead of patching, could you please find ways to simplify the logic to fix more general cases? If not, can you add comment to the end of each branch such as // end of if (s < -1) to make the code more readable.
e0acfda to
65df5d1
Compare
@apeforest Simplified the logic and refactored the code. Also added comments. Please review. |
apeforest
left a comment
There was a problem hiding this comment.
The change looks elegant! Thanks a lot for going the extra mile to make the code more clean and easy to read. Only one comment about unit test, otherwise it's good to ship.
03fa76f to
a3f995f
Compare
refactoring logic for indexing
a3f995f to
1625f06
Compare
refactoring logic for indexing
refactoring logic for indexing
refactoring logic for indexing
Description
This PR fixes nd.slice for erroneous output in the following case: begin=end ( fixes #13760 )
Added unit test for the same.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes