script::Module as a view. #21814

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.

Sign up for GitHub

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

Jump to bottom

Closed

zdevito wants to merge 11 commits into gh/zdevito/65/base from gh/zdevito/65/head

Contributor

zdevito commented Jun 15, 2019 •

edited

Loading

Stack from ghstack:

Add interface declarations to JIT #21853 Add interface declarations to JIT
script::Module as a view. #21814 script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

Differential Revision: D15839583


          script::Module as a view.

ce60ba5

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

zdevito requested review from ebetica, goldsborough and yf225 as code owners

June 15, 2019 02:02

pytorchbot added oncall: jit module: cpp module: internals module: pybind labels

This was referenced Jun 15, 2019

Remove lowered execution #21674

Closed

Make script::Method a value type #21675

Closed

zdevito added 3 commits

June 14, 2019 21:43


          Update on "script::Module as a view."

a0af5d3

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head


          Update on "script::Module as a view."

cf37235

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head


          Update on "script::Module as a view."

d3ceb16

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head

zdevito requested a review from suo

June 16, 2019 21:03


          Update on "script::Module as a view."

1f34903

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head

zdevito mentioned this pull request

Add interface declarations to JIT #21853

Merged

suo approved these changes

View reviewed changes

torch/csrc/jit/script/module.h Outdated

    
                }

                at::optional<size_t> find_offset(

                void get_or_add_slot(

Member

suo Jun 17, 2019

This seems more like set_or_add_slot

torch/csrc/jit/script/module.h

    
                      name, module->type(), module->module_object(), EntityType::MODULE);

                }

                void set_parameter(const std::string& name, at::Tensor v) {

Member

suo Jun 17, 2019

(future PR) we have a set_parameter but no set_buffer/set_attribute. Probably worth adding

torch/csrc/jit/script/module.h

    
                  return get_method("forward")(std::move(inputs));

                }

                void register_buffer(const std::string& name, autograd::Variable v) {

Member

suo Jun 17, 2019

May be worth dropping a note here about why buffers are treated as attributes

torch/csrc/jit/script/module.h Outdated

    
                }

                c10::ArrayRef<Slot> get_attributes() const {

                  return attributes_;

                std::vector<std::shared_ptr<Module>> get_modules() const {

Member

suo Jun 17, 2019

I don't think is makes sense for these to be shared_ptr anymore, since Module is a value type anyway

Contributor Author

zdevito Jun 18, 2019

I agree, but this is a big breaking API change for anyone using this API. Any call like foo.get_submodule("bar")->is_training() will need to change because the thing being returned is no longer a pointer. We should do it but it will need to be in a follow up PR to minimize the amount of API changes this one causes.

torch/csrc/jit/script/module.h

    
                  return nullptr;

                  return c10::nullopt;

                }

                std::shared_ptr<Module> find_module(const std::string& name) const {

Member

suo Jun 17, 2019

Same here

torch/csrc/jit/script/init.cpp Outdated

    
                        "_has_attribute",

                        [](Module& self, const std::string& name) -> bool {

                          return self.find_attribute(name);

                          return !!self.find_attribute(name);

Member

suo Jun 17, 2019

has_value?

zdevito mentioned this pull request

Add slot_list/slot_iterator #21874

Closed


          Update on "script::Module as a view."

2d0e2d0

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

We've changed the implementation of iterating over parameters/attributes/modules
many times, and each time we've had to slightly break backward compatibility.
This also changes the api to use a class (slot_list/iterator) to encapsulate
iteration of parameters/attributes/modules. This should allow us to make
implementation changes without affecting the user API.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head

zdevito mentioned this pull request

[JIT] Introduce NamedType #21835

Closed

zdevito added 5 commits

June 17, 2019 23:35


          Update on "script::Module as a view."

ebeb396

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

We've changed the implementation of iterating over parameters/attributes/modules
many times, and each time we've had to slightly break backward compatibility.
This also changes the api to use a class (slot_list/iterator) to encapsulate
iteration of parameters/attributes/modules. This should allow us to make
implementation changes without affecting the user API.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head


          Update on "script::Module as a view."

46a062c

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

We've changed the implementation of iterating over parameters/attributes/modules
many times, and each time we've had to slightly break backward compatibility.
This also changes the api to use a class (slot_list/iterator) to encapsulate
iteration of parameters/attributes/modules. This should allow us to make
implementation changes without affecting the user API.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head


          Update on "script::Module as a view."

d76be04

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

We've changed the implementation of iterating over parameters/attributes/modules
many times, and each time we've had to slightly break backward compatibility.
This also changes the api to use a class (slot_list/iterator) to encapsulate
iteration of parameters/attributes/modules. This should allow us to make
implementation changes without affecting the user API.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head


          Update on "script::Module as a view."

0a0e42e

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

We've changed the implementation of iterating over parameters/attributes/modules
many times, and each time we've had to slightly break backward compatibility.
This also changes the api to use a class (slot_list/iterator) to encapsulate
iteration of parameters/attributes/modules. This should allow us to make
implementation changes without affecting the user API.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head


          Update on "script::Module as a view."

ee0843d

script::Module as a view.

This moves all the module description logic (i.e. what members
of the class are parameters) into the ClassType for the module
itself. This allows us to remove all state in script::Module,
making it a stateless view of the class.

We've changed the implementation of iterating over parameters/attributes/modules
many times, and each time we've had to slightly break backward compatibility.
This also changes the api to use a class (slot_list/iterator) to encapsulate
iteration of parameters/attributes/modules. This should allow us to make
implementation changes without affecting the user API.

gh-metadata: pytorch pytorch 21814 gh/zdevito/65/head

facebook-github-bot closed this in

eda08b0

zou3519 deleted the gh/zdevito/65/head branch

June 18, 2019 21:01

zdevito added a commit to zdevito/ATen that referenced this pull request


          script::Module as a view. (#21814)

4e7c615

Summary:
Pull Request resolved: pytorch/pytorch#21814
ghimport-source-id: 49cfea6101ad9ca438600c465762e23252e05ff3

Test Plan: Imported from OSS

Differential Revision: D15839583

Pulled By: zdevito

fbshipit-source-id: ab4ef31a523b3ac1477aa7e6d4d9513e7408c560

facebook-github-bot added the merged label

Contributor

facebook-github-bot commented Jun 18, 2019

@zdevito merged this pull request in eda08b0.

mruberry added the Merged label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: cpp module: internals module: pybind oncall: jit