Skip to content

Remove unnecessary ATen/core/EnableNamedTensor.h#31117

Closed
zou3519 wants to merge 3 commits intogh/zou3519/222/basefrom
gh/zou3519/222/head
Closed

Remove unnecessary ATen/core/EnableNamedTensor.h#31117
zou3519 wants to merge 3 commits intogh/zou3519/222/basefrom
gh/zou3519/222/head

Conversation

@zou3519
Copy link
Contributor

@zou3519 zou3519 commented Dec 11, 2019

Stack from ghstack:

After this diff, we will have completely removed the named tensor
feature flagging. This means that named tensors are always on and that
there is no mechanism to turn them off. There should be no more follow-up
diffs.

I performed the deletion of the header with

find . -type f -print0 | xargs -0 sed -i '/#include
<ATen\/core\/EnableNamedTensor.h>/d'

Test Plan:

  • wait for CI

Differential Revision: D18934952

After this diff, we will have completely removed the named tensor
feature flagging. This means that named tensors are always on and that
there is no mechanism to turn them off. There should be no more follow-up
diffs.

I performed the deletion of the header with
```
find . -type f -print0 | xargs -0 sed -i '/#include
<ATen\/core\/EnableNamedTensor.h>/d'
```

Test Plan:
- wait for CI

[ghstack-poisoned]
@zou3519 zou3519 requested a review from apaszke as a code owner December 11, 2019 16:36
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Dec 11, 2019
After this diff, we will have completely removed the named tensor
feature flagging. This means that named tensors are always on and that
there is no mechanism to turn them off. There should be no more follow-up
diffs.

I performed the deletion of the header with
```
find . -type f -print0 | xargs -0 sed -i '/#include
<ATen\/core\/EnableNamedTensor.h>/d'
```

Test Plan:
- wait for CI

[ghstack-poisoned]
@zou3519 zou3519 requested review from gchanan, izdeby and nairbv and removed request for apaszke December 11, 2019 17:28
@@ -1,2 +0,0 @@
#pragma once
// We are working on removing the BUILD_NAMEDTENSOR flag from the codebase.
Copy link
Collaborator

@nairbv nairbv Dec 11, 2019

Choose a reason for hiding this comment

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

it's a bit unclear to me from github, is this removing the file or just the contents of the file? if not removing, then I'd think we still have one more PR to add to the stack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is removing the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

After this diff, we will have completely removed the named tensor
feature flagging. This means that named tensors are always on and that
there is no mechanism to turn them off. There should be no more follow-up
diffs.

I performed the deletion of the header with
```
find . -type f -print0 | xargs -0 sed -i '/#include
<ATen\/core\/EnableNamedTensor.h>/d'
```

Test Plan:
- wait for CI

Differential Revision: [D18934952](https://our.internmc.facebook.com/intern/diff/D18934952)

[ghstack-poisoned]
zou3519 added a commit that referenced this pull request Dec 12, 2019
After this diff, we will have completely removed the named tensor
feature flagging. This means that named tensors are always on and that
there is no mechanism to turn them off. There should be no more follow-up
diffs.

I performed the deletion of the header with
```
find . -type f -print0 | xargs -0 sed -i '/#include
<ATen\/core\/EnableNamedTensor.h>/d'
```

Test Plan:
- wait for CI

ghstack-source-id: e7593e6
Pull Request resolved: #31117
@kostmo
Copy link
Member

kostmo commented Dec 12, 2019

CircleCI build failures summary

As of commit b31cc46:

  • 1/1 failures introduced in this PR

Detailed failure analysis

One may explore the probable reasons each build failed interactively on the Dr. CI website.

1 failure not recognized by patterns:

Job Step
CircleCI pytorch_windows_build Build

This comment was automatically generated by Dr. CI.
Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

@zou3519
Copy link
Contributor Author

zou3519 commented Dec 12, 2019

I reran the windows build and it succeeded: https://app.circleci.com/jobs/github/pytorch/pytorch/3923679 so the build looks flaky.

@facebook-github-bot
Copy link
Contributor

@zou3519 merged this pull request in bcb0bb7.

@facebook-github-bot facebook-github-bot deleted the gh/zou3519/222/head branch December 16, 2019 15:18
wuhuikx pushed a commit to wuhuikx/pytorch that referenced this pull request Jan 30, 2020
Summary:
Pull Request resolved: pytorch#31117

After this diff, we will have completely removed the named tensor
feature flagging. This means that named tensors are always on and that
there is no mechanism to turn them off. There should be no more follow-up
diffs.

I performed the deletion of the header with
```
find . -type f -print0 | xargs -0 sed -i '/#include
<ATen\/core\/EnableNamedTensor.h>/d'
```

Test Plan: - wait for CI

Differential Revision: D18934952

Pulled By: zou3519

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

Labels

Merged 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