Skip to content

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Mar 26, 2019

Stack from ghstack:

Currently it holds a symbol whose unqualified name is the name of the
class. This will get confusing when there are multiple possible registries,
and it makes getting the class type from the object difficult.
The pointer to the class is only 4 more bytes so this patch just puts
it in the object.

Differential Revision: D14613510

Currently it holds a symbol whose unqualified name is the name of the
class. This will get confusing when there are multiple possible registries,
and it makes getting the class type from the object difficult.
The pointer to the class is only 4 more bytes so this patch just puts
it in the object.
Copy link
Member

@suo suo left a comment

Choose a reason for hiding this comment

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

lgtm. I thought there would be a circular dependency issue but apparently not lol.

Also, this look oddly formatted (lots of extra lines added and stuff). Run clang-format?

facebook-github-bot pushed a commit that referenced this pull request Apr 5, 2019
…18468)

Summary:
Pull Request resolved: #18468
ghimport-source-id: d4b41c5

Stack from [ghstack](https://github.com/ezyang/ghstack):
* #18469 Create Object that represents a Module
* **#18468 slots with explicit value/setValue make more sense in future patches**
* #18467 Make Object hold its ClassType
* #18379 Enforce single parent for script submodules
* #18378 Unify namespace of script::Module
* #18314 Add ability to specialize class types to ArgumentSpec
* #18226 Add Slot type to abstract the raw pointers being used for slots.

Reviewed By: suo

Differential Revision: D14613509

fbshipit-source-id: 9f2208d0efd01465c78cebdc3e8365a9e0adf9ff
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 091acb0.

zdevito added a commit to zdevito/ATen that referenced this pull request Apr 5, 2019
Summary:
Pull Request resolved: pytorch/pytorch#18467
ghimport-source-id: d51bdd64d2529d08c634c58df1a0870b54ad49fb

Stack from [ghstack](https://github.com/ezyang/ghstack):
* #18469 Create Object that represents a Module
* #18468 slots with explicit value/setValue make more sense in future patches
* **#18467 Make Object hold its ClassType**
* #18379 Enforce single parent for script submodules
* #18378 Unify namespace of script::Module
* #18314 Add ability to specialize class types to ArgumentSpec
* #18226 Add Slot type to abstract the raw pointers being used for slots.

Currently it holds a symbol whose unqualified name is the name of the
class. This will get confusing when there are multiple possible registries,
and it makes getting the class type from the object difficult.
The pointer to the class is only 4 more bytes so this patch just puts
it in the object.

Reviewed By: suo

Differential Revision: D14613510

fbshipit-source-id: b35175ba4be83d2522deaa6dad5070d6ec691fed
facebook-github-bot pushed a commit that referenced this pull request Apr 6, 2019
Summary:
Pull Request resolved: #18469
ghimport-source-id: 73cb8b5

Stack from [ghstack](https://github.com/ezyang/ghstack):
* **#18469 Create Object that represents a Module**
* #18468 slots with explicit value/setValue make more sense in future patches
* #18467 Make Object hold its ClassType
* #18379 Enforce single parent for script submodules
* #18378 Unify namespace of script::Module
* #18314 Add ability to specialize class types to ArgumentSpec
* #18226 Add Slot type to abstract the raw pointers being used for slots.

This changes the underlying storage for script::Module to hold
a ivalue::Object which has slots for all the parameters and attributes.

NamedIValue and Slot are now merged together into one class Slot that stores
the tuple (ivalue::Object, offset) and can be used to read the name, type,
or value of the slot and also to set the value. This cleans up a bunch
of client uses.

This PR does not actually use the module object in any generated code.
A future PR will switch how code is generated to treat modules as
first class.

Differential Revision: D14613508

fbshipit-source-id: d853a7559f58d244de2ef54a781427fcd1060ed0
zhangguanheng66 pushed a commit to zhangguanheng66/pytorch that referenced this pull request May 6, 2019
Summary:
Pull Request resolved: pytorch#18467
ghimport-source-id: d51bdd6

Stack from [ghstack](https://github.com/ezyang/ghstack):
* pytorch#18469 Create Object that represents a Module
* pytorch#18468 slots with explicit value/setValue make more sense in future patches
* **pytorch#18467 Make Object hold its ClassType**
* pytorch#18379 Enforce single parent for script submodules
* pytorch#18378 Unify namespace of script::Module
* pytorch#18314 Add ability to specialize class types to ArgumentSpec
* pytorch#18226 Add Slot type to abstract the raw pointers being used for slots.

Currently it holds a symbol whose unqualified name is the name of the
class. This will get confusing when there are multiple possible registries,
and it makes getting the class type from the object difficult.
The pointer to the class is only 4 more bytes so this patch just puts
it in the object.

Reviewed By: suo

Differential Revision: D14613510

fbshipit-source-id: b35175ba4be83d2522deaa6dad5070d6ec691fed
zhangguanheng66 pushed a commit to zhangguanheng66/pytorch that referenced this pull request May 6, 2019
…ytorch#18468)

Summary:
Pull Request resolved: pytorch#18468
ghimport-source-id: d4b41c5

Stack from [ghstack](https://github.com/ezyang/ghstack):
* pytorch#18469 Create Object that represents a Module
* **pytorch#18468 slots with explicit value/setValue make more sense in future patches**
* pytorch#18467 Make Object hold its ClassType
* pytorch#18379 Enforce single parent for script submodules
* pytorch#18378 Unify namespace of script::Module
* pytorch#18314 Add ability to specialize class types to ArgumentSpec
* pytorch#18226 Add Slot type to abstract the raw pointers being used for slots.

Reviewed By: suo

Differential Revision: D14613509

fbshipit-source-id: 9f2208d0efd01465c78cebdc3e8365a9e0adf9ff
zhangguanheng66 pushed a commit to zhangguanheng66/pytorch that referenced this pull request May 6, 2019
Summary:
Pull Request resolved: pytorch#18469
ghimport-source-id: 73cb8b5

Stack from [ghstack](https://github.com/ezyang/ghstack):
* **pytorch#18469 Create Object that represents a Module**
* pytorch#18468 slots with explicit value/setValue make more sense in future patches
* pytorch#18467 Make Object hold its ClassType
* pytorch#18379 Enforce single parent for script submodules
* pytorch#18378 Unify namespace of script::Module
* pytorch#18314 Add ability to specialize class types to ArgumentSpec
* pytorch#18226 Add Slot type to abstract the raw pointers being used for slots.

This changes the underlying storage for script::Module to hold
a ivalue::Object which has slots for all the parameters and attributes.

NamedIValue and Slot are now merged together into one class Slot that stores
the tuple (ivalue::Object, offset) and can be used to read the name, type,
or value of the slot and also to set the value. This cleans up a bunch
of client uses.

This PR does not actually use the module object in any generated code.
A future PR will switch how code is generated to treat modules as
first class.

Differential Revision: D14613508

fbshipit-source-id: d853a7559f58d244de2ef54a781427fcd1060ed0
@ezyang ezyang deleted the gh/zdevito/5/head branch May 30, 2019 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants