Skip to content

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Nov 8, 2019

Stack from ghstack:

This removes a lot of the private methods on torch._C.ScriptModule,
and instead implements functionality in terms of slot_dict_impl views
to implement _parameter, _buffers, and _modules in nn.Module.

A followup PR should also remove the _register_attribute,
_register_module, and _register_parameter methods, but this requires
more refactoring of the way tracing creates modules and replication
for data parallel works.

Differential Revision: D18387963

This removes a lot of the private methods on torch._C.ScriptModule,
and instead implements functionality in terms of slot_dict_impl views
to implement _parameter, _buffers, and _modules in nn.Module.

A followup PR should also remove the _register_attribute,
_register_module, and _register_parameter methods, but this requires
more refactoring of the way tracing creates modules and replication
for data parallel works.
@zdevito zdevito requested a review from apaszke as a code owner November 8, 2019 01:43
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 8, 2019
@zdevito zdevito requested a review from ZolotukhinM November 8, 2019 01:44
[jit] Simplify ScriptModule bindings.

This removes a lot of the private methods on torch._C.ScriptModule,
and instead implements functionality in terms of slot_dict_impl views
to implement _parameter, _buffers, and _modules in nn.Module.

A followup PR should also remove the _register_attribute,
_register_module, and _register_parameter methods, but this requires
more refactoring of the way tracing creates modules and replication
for data parallel works.

gh-metadata: pytorch pytorch 29432 gh/zdevito/132/head
Copy link

@ZolotukhinM ZolotukhinM left a comment

Choose a reason for hiding this comment

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

Looks good!

@facebook-github-bot
Copy link
Contributor

@zdevito merged this pull request in 4e4e29a.

@facebook-github-bot facebook-github-bot deleted the gh/zdevito/132/head branch November 15, 2019 15:17
@NegatioN
Copy link
Contributor

I know this is probably the wrong place to ask, but I've gotta know.

Since the plan is to also factor out _register_parameter(), I'm guessing it's to be factored out of torch::jit::script::Module completely?

Would this still be possible to achieve through torch::jit::script::Module.apply() or is the idea to completely remove the option to modify the underlying tensors/parameters?

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants