Skip to content

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Nov 12, 2019

Stack from ghstack:

Our intention is to merge the static distinction between Tensor and
Variable. Ordinarily, this would entail merging the methods of Tensor
and Variable. But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class. So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users). This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang [email protected]

Differential Revision: D18496169

Our intention is to merge the static distinction between Tensor and
Variable.  Ordinarily, this would entail merging the methods of Tensor
and Variable.  But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class.  So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users).  This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang <[email protected]>

[ghstack-poisoned]
…functions."

Our intention is to merge the static distinction between Tensor and
Variable.  Ordinarily, this would entail merging the methods of Tensor
and Variable.  But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class.  So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users).  This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang <[email protected]>

[ghstack-poisoned]
@ezyang ezyang requested review from gchanan and yf225 November 12, 2019 21:11
ezyang added a commit that referenced this pull request Nov 12, 2019
Our intention is to merge the static distinction between Tensor and
Variable.  Ordinarily, this would entail merging the methods of Tensor
and Variable.  But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class.  So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users).  This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang <[email protected]>

ghstack-source-id: 06451a9
Pull Request resolved: #29665
…functions."

Our intention is to merge the static distinction between Tensor and
Variable.  Ordinarily, this would entail merging the methods of Tensor
and Variable.  But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class.  So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users).  This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang <[email protected]>

[ghstack-poisoned]
@kostmo
Copy link
Member

kostmo commented Nov 13, 2019

CircleCI build failures summary

As of commit 1a08c82:

  • 1/2 broken upstream at merge base 6d54c5d (see grid view)
    • You may want to rebase on the viable/strict branch (see age history):
      git fetch viable/strict
      git rebase viable/strict
      
  • 1/2 failures introduced in this PR
  • 0/2 recognized as flaky

Here are the reasons each build failed:

Job Step Log excerpt
pytorch_xla_linux_xenial_py3_6_clang7_build Build Failed to run '['/var/lib/jenkins/workspace/xla/scripts/generate_code.sh']'
pytorch_linux_xenial_py3_6_gcc5_4_test Test main()\':\n/tmp/torch_extensions/test_compilation_error_formatting/main.cpp:2:23: error:

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.

This comment has been revised 1 time(s).

…functions."

Our intention is to merge the static distinction between Tensor and
Variable.  Ordinarily, this would entail merging the methods of Tensor
and Variable.  But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class.  So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users).  This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang <[email protected]>

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

[ghstack-poisoned]
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

You didn't changed the moved functions (appart from arguments massaging) right?

@ezyang
Copy link
Contributor Author

ezyang commented Nov 15, 2019

You didn't changed the moved functions (appart from arguments massaging) right?

Yep, no changes.

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in 1ab2f04.

ezyang added a commit to ezyang/pytorch that referenced this pull request Nov 19, 2019
Our intention is to merge the static distinction between Tensor and
Variable.  Ordinarily, this would entail merging the methods of Tensor
and Variable.  But there are a lot of "private"-ish methods on Variable
that we don't actually want to dump onto the Tensor class.  So, as prep
work, we move all of those methods off of Variable and into
the torch::autograd::impl namespace (impl as in, please don't use this
end users).  This ends up being a fairly large patch because all of
the call sites have to play ball too.

While I was on the topic, I also moved any of the touched functions into
the C++ file, so that modifying them would not trigger a recompilation of
all of torch.

Signed-off-by: Edward Z. Yang <[email protected]>

ghstack-source-id: eb15fb8
Pull Request resolved: pytorch#29665
@facebook-github-bot facebook-github-bot deleted the gh/ezyang/531/head branch November 22, 2019 15:17
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.

7 participants