Conversation
facebook-github-bot
left a comment
There was a problem hiding this comment.
@apaszke has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
thread locals have been causing trouble for me over at #8792 because once python-less libtorch.so is split out from python-full _C.so, Windows doesn't support exposing thread_local variables across the shared-object boundary. Will this PR introduce another occurrence of this issue? |
|
@anderspapitto The thread local is not exported (it is not even in a header) so there should be no problem. |
| namespace torch { namespace autograd { | ||
| // Helper methods for working with Attributes (torch/csrc/jit/attributes.h) | ||
|
|
||
| at::Tensor maybeUnwrapVar(const at::Tensor& t) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| if (!THPVariable_Check(item)) return false; | ||
| auto & var = reinterpret_cast<THPVariable*>(item)->cdata; | ||
| return torch::jit::tracer::isTracing(var); | ||
| return var.dim() == 0 && torch::jit::tracer::getValueTrace(var); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| //////////////////////////////////////////////////////////////////////////////// | ||
| thread_local ArgumentStash ArgumentStash::stash; | ||
|
|
||
| void ArgumentStash::stashIntListElem(const std::string& arg_name, size_t size, size_t idx, const Variable& var) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
zdevito
left a comment
There was a problem hiding this comment.
This looks good! Cleans up a lot of stuff.
| if (!THPVariable_Check(item)) return false; | ||
| auto & var = reinterpret_cast<THPVariable*>(item)->cdata; | ||
| return torch::jit::tracer::isTracing(var); | ||
| return var.dim() == 0 && torch::jit::tracer::getValueTrace(var); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| //////////////////////////////////////////////////////////////////////////////// | ||
| thread_local ArgumentStash ArgumentStash::stash; | ||
|
|
||
| void ArgumentStash::stashIntListElem(const std::string& arg_name, size_t size, size_t idx, const Variable& var) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@pytorchbot retest this please |
|
I would change the name of the function, but this would mean a whole roundtrip through the internal CI, which I'd like to avoid. We can fix it up later. |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@apaszke has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@apaszke has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@apaszke has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: As in the title. Lets us simplify a lot of code. Depends on pytorch#9363, so please review only the last commit. zdevito Pull Request resolved: pytorch#9414 Reviewed By: zdevito Differential Revision: D8836496 Pulled By: apaszke fbshipit-source-id: 9b3c3d1f001a9dc522f8478abc005b6b86cfa3e3
Summary: As in the title. Lets us simplify a lot of code. Depends on pytorch#9363, so please review only the last commit. zdevito Pull Request resolved: pytorch#9414 Reviewed By: zdevito Differential Revision: D8836496 Pulled By: apaszke fbshipit-source-id: 9b3c3d1f001a9dc522f8478abc005b6b86cfa3e3
As in the title. Lets us simplify a lot of code.
Depends on #9363, so please review only the last commit.
@zdevito