Skip to content

[jit][script] fix off-by-one bug in open-ended slicing#10286

Closed
suo wants to merge 3 commits intomasterfrom
fix_slice
Closed

[jit][script] fix off-by-one bug in open-ended slicing#10286
suo wants to merge 3 commits intomasterfrom
fix_slice

Conversation

@suo
Copy link
Member

@suo suo commented Aug 7, 2018

Previously, tensor[i:] was transformed to tensor[i:-1]. This incorrectly leaves off the last element. Noticed this when implementing slicing for list types.

@suo suo added the oncall: jit Add this issue/PR to JIT oncall triage queue label Aug 7, 2018
@suo suo requested review from jamesr66a and zdevito August 7, 2018 00:36
@suo
Copy link
Member Author

suo commented Aug 7, 2018

@pytorchbot retest this please


const auto has_end = slice.end().present();
if (has_end) {
// If the user specified an `end` index, pass it down

This comment was marked as off-topic.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

michaelsuo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

if (has_end) {
// If the user specified an `end` index, pass it down
args.emplace_back(loc, "end", emitExpr(Expr(slice.end().get()), identity));
}

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@apaszke apaszke deleted the fix_slice branch August 7, 2018 14:04
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary:
Previously, `tensor[i:]` was transformed to `tensor[i:-1]`. This incorrectly leaves off the last element. Noticed this when implementing slicing for list types.
Pull Request resolved: pytorch#10286

Differential Revision: D9193292

Pulled By: michaelsuo

fbshipit-source-id: df372b815f9a3b8029830dd9e8769f9985a890e7
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants