Skip to content

[TorchTidy] Adding Layout support and exposing TensorMetadata#81155

Closed
Gamrix wants to merge 6 commits intogh/gamrix/81/basefrom
gh/gamrix/81/head
Closed

[TorchTidy] Adding Layout support and exposing TensorMetadata#81155
Gamrix wants to merge 6 commits intogh/gamrix/81/basefrom
gh/gamrix/81/head

Conversation

@Gamrix
Copy link
Contributor

@Gamrix Gamrix commented Jul 9, 2022

Stack from ghstack:

TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

TensorMetadata will be used later to hold various metadata fields including sizes and strides.

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 9, 2022

🔗 Helpful links

❌ 1 New Failures

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

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

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

See GitHub Actions build pull / linux-bionic-py3.7-clang9 / test (dynamo, 2, 2, linux.2xlarge) (1/1)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

2022-07-21T18:13:56.0148312Z AssertionError: assert in []
2022-07-21T18:13:56.0144449Z   File "/opt/conda/lib/python3.7/site-packages/torch/_prims/__init__.py", line 442, in forward
2022-07-21T18:13:56.0144784Z     return _prim(*args, **kwargs)
2022-07-21T18:13:56.0145237Z   File "/opt/conda/lib/python3.7/site-packages/torch/_ops.py", line 49, in __call__
2022-07-21T18:13:56.0145591Z     return self._op(*args, **kwargs or {})
2022-07-21T18:13:56.0146098Z   File "/opt/conda/lib/python3.7/site-packages/torch/utils/_python_dispatch.py", line 74, in wrapped
2022-07-21T18:13:56.0146462Z     return f(self, *args, **kwargs)
2022-07-21T18:13:56.0146817Z   File "/var/lib/jenkins/workspace/test/test_ops.py", line 1473, in __torch_dispatch__
2022-07-21T18:13:56.0147221Z     def __torch_dispatch__(self, func, types, args=(), kwargs=None):
2022-07-21T18:13:56.0147608Z   File "/var/lib/jenkins/workspace/test/test_ops.py", line 1467, in check_inplace_view
2022-07-21T18:13:56.0147979Z     assert torch.Tag.inplace_view in func.tags
2022-07-21T18:13:56.0148312Z AssertionError: assert <Tag.inplace_view: 4> in []
2022-07-21T18:13:56.0148743Z  +  where <Tag.inplace_view: 4> = <class 'torch.Tag'>.inplace_view
2022-07-21T18:13:56.0149134Z  +    where <class 'torch.Tag'> = torch.Tag
2022-07-21T18:13:56.0149564Z  +  and   [] = <OpOverload(op='prims.copy_to', overload='default')>.tags
2022-07-21T18:13:56.0544968Z =============================== warnings summary ===============================
2022-07-21T18:13:56.0545972Z ../../../../../opt/conda/lib/python3.7/site-packages/_pytest/config/__init__.py:1129
2022-07-21T18:13:56.0547045Z   /opt/conda/lib/python3.7/site-packages/_pytest/config/__init__.py:1129: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: hypothesis
2022-07-21T18:13:56.0547538Z     self._mark_plugins_for_rewrite(hook)
2022-07-21T18:13:56.0547674Z 
2022-07-21T18:13:56.0551826Z ../../../../../opt/conda/lib/python3.7/site-packages/torch/testing/_internal/common_methods_invocations.py:18025
2022-07-21T18:13:56.0552546Z ../../../../../opt/conda/lib/python3.7/site-packages/torch/testing/_internal/common_methods_invocations.py:18025

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.

Gamrix added a commit that referenced this pull request Jul 9, 2022
TensorMetadata will be used later to hold various metadata fields including sizes and strides.

ghstack-source-id: 120971a
Pull Request resolved: #81155
@albanD albanD removed their request for review July 11, 2022 14:20
…ata"


TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

[ghstack-poisoned]
…ata"


TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

[ghstack-poisoned]
Gamrix added a commit that referenced this pull request Jul 12, 2022
TensorMetadata will be used later to hold various metadata fields including sizes and strides.

ghstack-source-id: a614b55
Pull Request resolved: #81155
@soulitzer soulitzer removed their request for review July 14, 2022 13:37
…ata"


TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

[ghstack-poisoned]
@Gamrix Gamrix requested a review from robieta July 14, 2022 21:46
Gamrix added a commit that referenced this pull request Jul 14, 2022
TensorMetadata will be used later to hold various metadata fields including sizes and strides.

ghstack-source-id: e20425d
Pull Request resolved: #81155
Copy link
Contributor

@robieta robieta left a comment

Choose a reason for hiding this comment

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

LGTM

…ata"


TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

[ghstack-poisoned]
Gamrix added a commit that referenced this pull request Jul 20, 2022
TensorMetadata will be used later to hold various metadata fields including sizes and strides.

ghstack-source-id: 6c21b14
Pull Request resolved: #81155
…ata"


TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

[ghstack-poisoned]
@Gamrix
Copy link
Contributor Author

Gamrix commented Jul 21, 2022

I have already rebased on viable/strict and the test failure seems to stay.

The Torchdynamo test failure seems to be flaky and unrelated to this diff:
Example run with same issue when on master:
c949788
https://pipelines.actions.githubusercontent.com/serviceHosts/7d146c05-69c3-4c20-a0e7-818111670117/_apis/pipelines/1/runs/2132305/signedlogcontent/238?urlExpires=2022-07-21T18%3A37%3A51.3174590Z&urlSigningMethod=HMACV1&urlSignature=ZkRl3AfwAoOoFphAbLyxNFq5stoJQYZzEbnNh%2B5x7dk%3D

@Gamrix
Copy link
Contributor Author

Gamrix commented Jul 21, 2022

@pytorchbot merge --force

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 21, 2022

❌ 🤖 pytorchbot command failed:

@pytorchbot merge: error: argument -f/--force: expected one argument

usage: @pytorchbot merge [-g | -f FORCE | -l]

Try @pytorchbot --help for more info.

@Gamrix
Copy link
Contributor Author

Gamrix commented Jul 21, 2022

@pytorchbot --help

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 21, 2022

PyTorchBot Help

usage: @pytorchbot [-h] {merge,revert,rebase,label} ...

In order to invoke the bot on your PR, include a line that starts with
@pytorchbot anywhere in a comment. That line will form the command; no
multi-line commands are allowed. 

Example:
    Some extra context, blah blah, wow this PR looks awesome

    @pytorchbot merge

optional arguments:
  -h, --help            Show this help message and exit.

command:
  {merge,revert,rebase,label}
    merge               Merge a PR
    revert              Revert a PR
    rebase              Rebase a PR
    label               Add label to a PR

Merge

usage: @pytorchbot merge [-g | -f FORCE | -l]

Merge an accepted PR, subject to the rules in .github/merge_rules.json.
By default, this will wait for all required checks to succeed before merging.

optional arguments:
  -g, --green           Merge when *all* status checks pass.
  -f FORCE, --force FORCE
                        Merge without checking anything. This requires a reason for auditting purpose, for example:
                        `@pytorchbot merge -f '[MINOR] Fix lint. Expecting all PR tests to pass'`
                        The reason must be longer than 2 words. ONLY USE THIS FOR CRITICAL FAILURES.
  -l, --land-checks     Merge with land time checks. This will create a new branch with your changes rebased on viable/strict and run additional tests (EXPERIMENTAL)

Revert

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

Revert a merged PR. This requires that you are a Meta employee.

Example:
  @pytorchbot revert -m="This is breaking tests on trunk. hud.pytorch.org/" -c=nosignal

optional arguments:
  -m MESSAGE, --message MESSAGE
                        The reason you are reverting, will be put in the commit message. Must be longer than 3 words.
  -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]

Rebase a PR. Rebasing defaults to the default branch of pytorch (master).
You, along with any member of the pytorch organization, can rebase your PR.

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

Label

usage: @pytorchbot label labels [labels ...]

Adds label to a PR

positional arguments:
  labels  Labels to add to given Pull Request

@Gamrix
Copy link
Contributor Author

Gamrix commented Jul 21, 2022

@pytorchbot merge -f 'The torchdynamo test failure is due to a flaky test, see PR for more details`

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 21, 2022

❌ 🤖 pytorchbot command failed:

Got EOF while in a quoted string```
Try `@pytorchbot --help` for more info.

@Gamrix
Copy link
Contributor Author

Gamrix commented Jul 21, 2022

@pytorchbot merge -f 'The torchdynamo test failure is due to a flaky test, see PR for more details'

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

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

facebook-github-bot pushed a commit that referenced this pull request Jul 22, 2022
#81155)

Summary:
TensorMetadata will be used later to hold various metadata fields including sizes and strides.

This is basically me separating the following diff into its logical components after they all got smushed together
#80072

Pull Request resolved: #81155
Approved by: https://github.com/robieta

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

Reviewed By: jeanschmidt

Differential Revision: D38066982

Pulled By: jeanschmidt

fbshipit-source-id: a583ea42e0a3ed438923a1bd4ccd9c16e2ab9f69
@facebook-github-bot facebook-github-bot deleted the gh/gamrix/81/head branch July 25, 2022 14:18
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.

4 participants