Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't generate named tensor methods to {Tensor,TensorMethods}.h #25022

Closed
wants to merge 2 commits into from

Conversation

zou3519
Copy link
Contributor

@zou3519 zou3519 commented Aug 22, 2019

Stack from ghstack:

Don't generate named tensor methods to {Tensor,TensorMethods}.h when
BUILD_NAMEDTENSOR=0 in an attempt to prevent fewer merge conflicts.

We can either fix the root cause by solving
#24931, or, in the short term,
go through with this (hacky) solution.

Here's what happens:

  1. We filter out all named tensor declarations if BUILD_NAMEDTENSOR=0 so
    that they never hit the codegen.
  2. What is checked in as {Tensor,TensorMethods}.h comes from a
    BUILD_NAMEDTENSOR=0 build.
  3. When BUILD_NAMEDTENSOR=1, then GEN_TO_SOURCE is automatically set to
    1. When working on a named tensor build, one must remember not to
      check in the generated code.

Test Plan:

  • run ci [namedtensor ci]

Don't generate named tensor methods to {Tensor,TensorMethods}.h when
BUILD_NAMEDTENSOR=0 in an attempt to prevent fewer merge conflicts.

We can either fix the root cause by solving
#24931, or, in the short term,
go through with this (hacky) solution.

Here's what happens:
1) We filter out all named tensor declarations if BUILD_NAMEDTENSOR=0 so
   that they never hit the codegen.
2) What is checked in as {Tensor,TensorMethods}.h comes from a
   BUILD_NAMEDTENSOR=0 build.
3) When BUILD_NAMEDTENSOR=1, then GEN_TO_SOURCE is automatically set to
   1. When working on a named tensor build, one must remember not to
   check in the generated code.

Test Plan:
- run ci [namedtensor ci]
…ds}.h"

Don't generate named tensor methods to {Tensor,TensorMethods}.h

Don't generate named tensor methods to {Tensor,TensorMethods}.h when
BUILD_NAMEDTENSOR=0 in an attempt to prevent fewer merge conflicts.

We can either fix the root cause by solving
#24931, or, in the short term,
go through with this (hacky) solution.

Here's what happens:
1) We filter out all named tensor declarations if BUILD_NAMEDTENSOR=0 so
   that they never hit the codegen.
2) What is checked in as {Tensor,TensorMethods}.h comes from a
   BUILD_NAMEDTENSOR=0 build.
3) When BUILD_NAMEDTENSOR=1, then GEN_TO_SOURCE is automatically set to
   1. When working on a named tensor build, one must remember not to
   check in the generated code.

Test Plan:
- run ci [namedtensor ci]

gh-metadata: pytorch pytorch 25022 gh/zou3519/116/head
Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

let's hold off on this temporarily until we can figure out if we can just remove the src checkin.

@@ -45,6 +45,9 @@
help='reinterpret CUDA as ROCm/HIP and adjust filepaths accordingly')
options = parser.parse_args()
gen_to_source = os.environ.get('GEN_TO_SOURCE') # update source directly as part of gen
if BUILD_NAMEDTENSOR:
# see Note [Named tensor build codegen]
gen_to_source = True
Copy link
Contributor

Choose a reason for hiding this comment

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

you should just change the default so it's true if BUILD_NAMEDTENSOR is on -- you should still be able to turn it off explicitly.

@zou3519 zou3519 closed this Sep 6, 2019
@facebook-github-bot facebook-github-bot deleted the gh/zou3519/116/head branch October 28, 2019 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: internals Related to internal abstractions in c10 and ATen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants