Skip to content

Conversation

@liqunfu
Copy link
Collaborator

@liqunfu liqunfu commented Jun 15, 2019

handle slice with negative indices and indices exceeding tensor dimensions.

@pytorchbot pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: onnx Related to torch.onnx labels Jun 15, 2019
@zhangguanheng66 zhangguanheng66 requested a review from suo June 15, 2019 22:33
@zhangguanheng66 zhangguanheng66 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 15, 2019
@spandantiwari spandantiwari requested a review from houseroad June 18, 2019 20:20
@houseroad houseroad requested a review from spandantiwari June 19, 2019 18:09
Copy link

@spandantiwari spandantiwari left a comment

Choose a reason for hiding this comment

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

LGTM.

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.

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

Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

_set_opset_version(9)
x = torch.ones(1, 3)
graph, _, __ = utils._model_to_graph(SliceModule(), (x, ),
graph, _, __ = utils._model_to_graph(NarrowModule(), (x, ),
Copy link
Member

Choose a reason for hiding this comment

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

nit: __ => _

also can you change all the __ to _?

int64_t length = end - start;
if (length < 0 || start > updated_val.sizes()[axis] - length)
return c10::nullopt;
updated_val = at::narrow(updated_val, axis, start, length);
Copy link
Member

Choose a reason for hiding this comment

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

you may also want to try at::slice

@houseroad
Copy link
Member

will merge this pr first, feel free to open another pr to address nits.

@facebook-github-bot
Copy link
Contributor

@houseroad merged this pull request in f9b3989.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: onnx Related to torch.onnx oncall: jit Add this issue/PR to JIT oncall triage queue open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants