[jit] Add module attributes by driazati · Pull Request #17309 · pytorch/pytorch

driazati · 2019-02-20T19:27:06Z

Similar to nn.Parameters, this PR lets you store any IValue on a module as an attribute on a ScriptModule (only from the Python front-end currently). To mark something as an attribute, it should wrapped in jit.Attribute(value, type) (ex. self.table = torch.jit.Attribute(table, Dict[str, torch.Tensor]))

Followup Work:

(de)serializing for use in C++
change self.training to be a bool attribute instead of a buffer
mutable attributes
string frontend support
documentation

driazati · 2019-02-20T21:53:01Z

@pytorchbot rebase this please

sidazhang · 2019-02-21T04:29:52Z

A few questions here:

What sort of types can be set as an attribute? Any IValue can be an attribute?
Can the type be mutated? i.e. Mutable dict?
Can you access the attribute in the C++ front end?

Basically

auto module = torch::load("./model.pt");
module.get_attribute("some_attribute_name")

driazati · 2019-02-21T19:26:25Z

This is pretty rough right now and won't have full support for a while, but:

Any IValue used in JIT (we don't plan to do some, e.g. Blob IValues)
Yes (eventually)
Yes (eventually)

zdevito

Great start -- I've put some comments inline about some issues that we need to resolve about figure out the right types for attributes, and how to structure the Module class.

zdevito · 2019-02-28T01:43:47Z

torch/csrc/jit/script/module.h

-  // to the slots where parameters are kept while also allow parameters
-  // to be reassigned
-  std::unique_ptr<at::Tensor> parameter;
+struct NamedInput {


It's not always an input, so NamedIValue would be a better term.

zdevito · 2019-02-28T01:46:18Z

torch/csrc/jit/script/module.h

    auto retval = graph_->copy();
-    for (auto inp : member_inputs) {
-      inputs.push_back(*inp);
+    for (auto inp : member_inputs_) {


The if statement here is going wreck havoc later. I think inputs/outputs should be flipped to IValue.

zdevito · 2019-02-28T01:55:47Z

torch/csrc/jit/script/module.h

  Module()
      : modules("Module"),
        parameters("Parameter"),
+        attributes("Attributes"),


We should be careful how we structure things here so that it mirrors how a python class works.
We probably want to drop the is_parameter flag and simply use the separate lists (attributes vs parameters) to decide whether something is a parameter. Things that were previously buffers should go in the attributes list.

zdevito · 2019-02-28T02:02:02Z

torch/jit/__init__.py

                return self.__getattr__('forward').graph
            return Module.__getattr__(self, attr)

+        def _is_attribute(self, attr, value):


This seems brittle because it has to look through every other potential place where getattr looks to make sure it doesn't fall into one of these categories. Is there a better way to structure this?

zdevito · 2019-02-28T02:03:13Z

torch/jit/__init__.py

        """.format(type(v).__name__, attr, constants)))


+def _get_type(value):


I am not a fan of adding this. It will need to be an exhaustive list, and we already have similar stuff in C++ for converting python values. It also cannot be done correctly: If a list is empty or a dict is empty we cannot infer the type. We should use the existing C++ code that figure out the type and IValue information from a python class rather than reimplement it here. We should also consider how we could inform the Module about the right type for things like empty dictionaries where the type is not inferrable.

zdevito

Looks pretty good. I have a few comments that should be addressed and then it is ready to land.

zdevito · 2019-03-01T02:55:40Z

torch/csrc/jit/export.cpp

In its current form, won't landing this will break serialization of buffers?

This was from an old commit and was fixed to use a flag

torch/csrc/jit/script/module.h

zdevito · 2019-03-06T21:34:22Z

torch/csrc/jit/script/init.cpp

      return std::make_shared<SimpleValue>(m.get_or_add_parameter(v->slot()));
+    } else if (NamedInput* v = module->find_attribute(field)) {
+      return std::make_shared<SimpleValue>(
+          m.get_or_add_attribute(v->type, v->slot()));


get_or_add_attribute should take a NamedInput* rather than both a type and a IValue*. Otherwise it is too easy to accidentally pass the wrong type.

zdevito · 2019-03-06T21:40:54Z

torch/csrc/jit/script/init.cpp

    // python method. If so return this as a python value.
    py::object py_module = py::cast(module);
    if (py::object attr = py::getattr(py_module, field.c_str(), py::none())) {
+      if (py::isinstance(


Attributes should be registered during the init process. Doing it lazily here does not follow the same pattern of parameters, and it prevents a user from having an attribute exist even when a method doesn't reference it. I see trouble with this when, for instance, someone uses a non-python way of defining a Method and it can't see the attributes because they are in a Python-only state at the moment.

facebook-github-bot

@driazati has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

@driazati has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

houseroad · 2019-03-07T20:23:50Z

@driazati this broke pytorch-xla build. cc: @ailzhang @suo

* upstream/master: (24 commits) Automatic update of fbcode/onnx to 96c58ceeacf0f2b73d752e413e4fd78787a12da3 (pytorch#17676) Set the default ONNX opset to the latest stable opset (i.e., 9) (pytorch#17736) Add module attributes (pytorch#17309) - refactoring serialization of ONNX initializers to be name-based (pytorch#17420) ONNX Export for Max and Average Pooling in CEIL_MODE use flake8-mypy (pytorch#17721) use fp16<->fp32 intrinsic (pytorch#17496) Implement a Caffe2 standalone LSTM operator (pytorch#17726) caffe2:libtorch_cuda depends on caffe2:caffe2_gpu (pytorch#17729) add tensor and cost inference functions (pytorch#17684) ONNX Export Narrow op Keep the dim_type of hinted shape as BATCH if possible (pytorch#17734) fix different round behavior on CPU and GPU pytorch#16498 (pytorch#17443) Warn about memory overlaps on expanded tensors (pytorch#17576) fix exp fam. formula refactor caffe2 operator constructors - 10/9 (pytorch#17659) Improve ONNX symbolic for logsoftmax and softmax (pytorch#17672) Enable using CMD when building cpp extensions on Windows Do not rename net boundary inputs/outputs during ssaRewrite. (pytorch#17545) Reapply D14078519 (pytorch#17596) ...

davidriazati added 2 commits February 20, 2019 11:12

attributes

b01fab4

cleanup

c9ae0c8

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Feb 20, 2019

davidriazati added 3 commits February 20, 2019 11:37

Cleanup

028a6fb

respect ordering

56ddc4e

test

21e3033

driazati marked this pull request as ready for review February 20, 2019 21:52

davidriazati added 2 commits February 20, 2019 14:20

Merge branch 'master' of https://github.com/pytorch/pytorch into attr

ff2696b

skip parameters / buffers / modules

7aec4c2

driazati and others added 3 commits February 21, 2019 13:34

Merge branch 'master' into attr

89a4666

almost with NP removed

ec89e70

remove NamedParameter

905459f

driazati requested review from ebetica, goldsborough and yf225 as code owners February 22, 2019 01:24

davidriazati added 12 commits February 21, 2019 17:25

fix test

e1edb69

cleanup

cbe5892

cleanup

3db9ecb

Merge branch 'master' of https://github.com/pytorch/pytorch into attr

9275649

cleanup

95dac76

fix some tests

c59146a

test

dddced0

Merge branch 'master' of https://github.com/pytorch/pytorch into attr

9445361

test_alexnet accept (why?)

8167069

rename NamedAttribute

e041522

rename

9c1876a

Fix tests

a8903ed

davidriazati added 3 commits February 25, 2019 12:46

Rename params() -> member_inputs()

0c2c37f

Merge

f74682c

Fix bad merge

bce97c5

driazati changed the title ~~[wip][jit] attributes~~ [jit] attributes Feb 26, 2019

driazati requested a review from zdevito February 26, 2019 22:52

driazati mentioned this pull request Feb 27, 2019

[jit] Attribute serialization #17423

Closed

zdevito reviewed Feb 28, 2019

View reviewed changes

davidriazati added 6 commits February 28, 2019 16:40

Add Module::buffers to keep serialization working

2f7f633

Merge branch 'master' of https://github.com/pytorch/pytorch into attr2

7b5ec69

Use torch.jit.Attribute to manually specify attributes

445d875

Remove Module::buffers

9765232

Cleanup

025d327

Fix copy_method

0a563a8

driazati changed the title ~~[jit] attributes~~ [jit] Add module attributes Mar 4, 2019

zdevito self-requested a review March 4, 2019 20:07

fix flake:

e5f94ce

zdevito approved these changes Mar 6, 2019

View reviewed changes

Register attributes in ScriptModule.__setattr__, address comments

5d8baee

facebook-github-bot reviewed Mar 6, 2019

View reviewed changes

driazati mentioned this pull request Mar 6, 2019

[JIT] Allow CPP frontend to access config attributes from the .pt #17683

Closed

Rename NamedInput -> NamedIValue

071e527

facebook-github-bot reviewed Mar 7, 2019

View reviewed changes

facebook-github-bot closed this in a2381fa Mar 7, 2019

pytorchbot added the merged label Mar 7, 2019

		""".format(type(v).__name__, attr, constants)))


		def _get_type(value):

Conversation

driazati commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

driazati commented Feb 20, 2019

Uh oh!

sidazhang commented Feb 21, 2019

Uh oh!

driazati commented Feb 21, 2019

Uh oh!

zdevito left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zdevito left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad commented Mar 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

driazati commented Feb 20, 2019 •

edited

Loading