Skip to content

Conversation

@drisspg
Copy link
Contributor

@drisspg drisspg commented Sep 1, 2022

Summary

I changed the calculation of offsets to add an extra element for bounding above. This invariant makes sense in the contiguous case but when ntesnor[i] is sliced like in this pr: #83736 this doesn't make semantic sense anymore. SO changing back, Borderline stampy

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 1, 2022

🔗 Helpful links

❌ 1 New Failures

As of commit 96f0fa1 (more details on the Dr. CI page):

Expand to see more
  • 1/1 failures introduced in this PR

🕵️‍♀️ 1 failure not recognized by patterns:

The following CI failures may be due to changes from the PR
Job Step
CircleCI Checks build Unknown

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@drisspg
Copy link
Contributor Author

drisspg commented Sep 1, 2022

@pytorchbot merge -l

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

pytorchmergebot pushed a commit that referenced this pull request Sep 1, 2022
# Summary
I changed the calculation of offsets to add an extra element for bounding above. This invariant makes sense in the contiguous case but when ntesnor[i] is sliced like in this pr: #83736 this doesn't make semantic sense anymore. SO changing back, Borderline stampy

Pull Request resolved: #84433
Approved by: https://github.com/mikaylagawarecki
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2022

Hey @drisspg.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@cpuhrsch cpuhrsch added the topic: not user facing topic category label Sep 6, 2022
facebook-github-bot pushed a commit that referenced this pull request Sep 6, 2022
Summary:
# Summary
I changed the calculation of offsets to add an extra element for bounding above. This invariant makes sense in the contiguous case but when ntesnor[i] is sliced like in this pr: #83736 this doesn't make semantic sense anymore. SO changing back, Borderline stampy

Pull Request resolved: #84433
Approved by: https://github.com/mikaylagawarecki

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/73cb6cf8ae355417e0e9b6b9614492b280f66ae7

Reviewed By: mehtanirav

Differential Revision: D39277388

Pulled By: drisspg

fbshipit-source-id: d0ba731b4698bee8595196f5ef9c414f10ce99b4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants