Skip to content

Conversation

@kshitij12345
Copy link
Collaborator

@kshitij12345 kshitij12345 commented May 31, 2022

at::for_blob used to short-circuit if first dim had size 0 or if number of dims was 1.
at::for_blob is used in _make_wrapper_subclass implementation and resulted in SubclassTensor having different stride for the above case.

import torch
from torch.testing._internal.composite_compliance import generate_cct
elem = torch.ones(0,).as_strided((0,), (2,), 0)
CCT = generate_cct()
ret = CCT(elem)
print(f"{ret.shape = }, {ret.stride() = }, {ret.elem.shape = }, {ret.elem.stride() = }")

Output Before

ret.shape = torch.Size([0]), ret.stride() = (1,), ret.elem.shape = torch.Size([0]), ret.elem.stride() = (2,)

Output After

ret.shape = torch.Size([0]), ret.stride() = (2,), ret.elem.shape = torch.Size([0]), ret.elem.stride() = (2,)

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 31, 2022

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit 06bfd09 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚


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.

@kshitij12345 kshitij12345 changed the title [composite compliance] _make_subclass_wrapper respect strides [composite compliance] _make_wrapper_subclass respect strides May 31, 2022
@kshitij12345 kshitij12345 requested a review from zou3519 June 2, 2022 15:06
@kshitij12345 kshitij12345 marked this pull request as ready for review June 2, 2022 15:06
@zou3519
Copy link
Contributor

zou3519 commented Jun 3, 2022

@pytorchbot merge this please

@pytorch-bot
Copy link

pytorch-bot bot commented Jun 3, 2022

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: unrecognized arguments: this please

usage: @pytorchbot {merge,revert,rebase,help} ...

Try @pytorchbot help for more info.

@zou3519
Copy link
Contributor

zou3519 commented Jun 3, 2022

@pytorchbot help

@pytorch-bot
Copy link

pytorch-bot bot commented Jun 3, 2022

PyTorchBot Help

usage: @pytorchbot {merge,revert,rebase,help} ...

command:
  {merge,revert,rebase,help}
    merge               Merge a PR
    revert              Revert a merged PR
    rebase              Rebase a PR
    help                Show help

Merge

usage: @pytorchbot merge [-g] [-a] [-f]

optional arguments:
  -g, --green      Merge when required status checks pass. Currently, we only
                   require lint and builds to pass.
  -a, --all-green  Merge when all status checks pass
  -f, --force      Merge without checking anything. ONLY USE THIS FOR CRITICAL
                   FAILURES.

Revert

usage: @pytorchbot revert -m MESSAGE
                          [-c {nosignal,ignoredsignal,landrace,weird,ghfirst}]

optional arguments:
  -m MESSAGE, --message MESSAGE
                        The reason you are reverting, will be put in the
                        commit message.
  -c {nosignal,ignoredsignal,landrace,weird,ghfirst}, --classification {nosignal,ignoredsignal,landrace,weird,ghfirst}
                        A machine-friendly classification of the revert
                        reason.

Rebase

usage: @pytorchbot rebase [-s | -b BRANCH]

optional arguments:
  -s, --stable          Rebase to viable/strict
  -b BRANCH, --branch BRANCH
                        Branch you would like to rebase to

For more info, consult the wiki.

@zou3519
Copy link
Contributor

zou3519 commented Jun 3, 2022

@pytorchbot merge -g

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2022

Hey @kshitij12345.
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.

@zou3519 zou3519 added the topic: not user facing topic category label Jun 3, 2022
facebook-github-bot pushed a commit that referenced this pull request Jun 4, 2022
#78520)

Summary:
`at::for_blob` used to short-circuit if first dim had size `0` or if number of dims was `1`.
`at::for_blob` is used in `_make_wrapper_subclass` implementation and resulted in `SubclassTensor` having different stride for the above case.

```python
import torch
from torch.testing._internal.composite_compliance import generate_cct
elem = torch.ones(0,).as_strided((0,), (2,), 0)
CCT = generate_cct()
ret = CCT(elem)
print(f"{ret.shape = }, {ret.stride() = }, {ret.elem.shape = }, {ret.elem.stride() = }")
```

Output Before
```
ret.shape = torch.Size([0]), ret.stride() = (1,), ret.elem.shape = torch.Size([0]), ret.elem.stride() = (2,)
```

Output After
```
ret.shape = torch.Size([0]), ret.stride() = (2,), ret.elem.shape = torch.Size([0]), ret.elem.stride() = (2,)
```

Pull Request resolved: #78520
Approved by: https://github.com/zou3519

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

Reviewed By: b0noI

Differential Revision: D36907490

fbshipit-source-id: 47089279fc23be916a48c3196cbc0143547b6046
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