remove uses of std::shared_ptr<Module> #21934
Conversation
pytorchbot
added
caffe2
oncall: jit
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
| const py::function& var_name_lookup_fn, | ||
| bool force_outplace, | ||
| const std::shared_ptr<script::Module>& self = nullptr); | ||
| script::Module* self = nullptr); |
There was a problem hiding this comment.
There are a couple places in this PR where we take Module*, I assume so that it can be null. I think it's a bit weird pass around a pointer to a view of a reference type…maybe c10::optional?
There was a problem hiding this comment.
The places where Module* are used are trying to avoid dependencies in headers. For instance, the tracer.h header is included inside of VariableType.h, a huge file. We are careful to avoid including Module.h because then it would rebuild a huge part of pytorch when modified. Unfortunately using c10::optional requires the definition to be present, so Module* is a workaround that does the same thing.
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
remove uses of std::shared_ptr<Module> gh-metadata: pytorch pytorch 21934 gh/zdevito/68/head
| # define C10_DEPRECATED_MESSAGE(message) [[deprecated(message)]] | ||
| #elif defined(__GNUC__) | ||
| # define C10_DEPRECATED __attribute__((deprecated)) | ||
| //# define C10_DEPRECATED __attribute__((deprecated)) |
There was a problem hiding this comment.
ouch was this change intentional part of this diff? I'm working on a diff fixing compiler warnings and it just cost me a few hours to figure out why I don't see them anymore after rebasing. The changes to this file should be reverted.
…orch#22291) Summary: Pull Request resolved: pytorch#22291 There was a race between landing the benchmark diff and pytorch#21934 from zdevito. This PR should fix the issue. Reviewed By: zdevito Differential Revision: D16023640 fbshipit-source-id: 931714352e656f045f9ef3cd17422db51b168384
Stack from ghstack:
Module is now a view of an ivalue object and has no other state. It no longer needs to be carried around in an std::shared_ptr and behaves more like IValue and Tensor. This PR updates the APIs to no longer return std::shared_ptr. It introduces
module_listto iterate over modules in the Module object which is an extension to the slot_list API that turns the Slot into a module on dereference. This is a BC breaking change because we are change core Module loading APIs to no longer return a shared_ptr. Users of the C++ part of the JIT will likely need to replace std::shared_ptr with Module in their code, and replace -> with . where those objects are used.Differential Revision: D15892401