Skip to content
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.

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

[JIT] Add __prepare_scriptable__ duck typing to allow replacing nn.modules with scriptable preparations (#45645) #49242

Closed

Conversation

SplitInfinity
Copy link

@SplitInfinity SplitInfinity commented Dec 11, 2020

Stack from ghstack:

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a __prepare_scriptable__ method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

Differential Revision: D25500303

…modules with scriptable preparations (#45645)

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Differential Revision: D24039990

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

[ghstack-poisoned]
SplitInfinity pushed a commit that referenced this pull request Dec 11, 2020
…modules with scriptable preparations (#45645)

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Differential Revision: D24039990

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

ghstack-source-id: 8cf5a7723a1fbde418800d55389ac2f588bb05bd
Pull Request resolved: #49242
@facebook-github-bot facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue labels Dec 11, 2020
…placing nn.modules with scriptable preparations (#45645)"


Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

Differential Revision: [D25500303](https://our.internmc.facebook.com/intern/diff/D25500303)

[ghstack-poisoned]
SplitInfinity pushed a commit that referenced this pull request Dec 16, 2020
…modules with scriptable preparations (#45645)

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Differential Revision: D24039990

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

ghstack-source-id: 05392dbb3b8af5810715f10c6255e220241fa306
Pull Request resolved: #49242
…placing nn.modules with scriptable preparations (#45645)"


Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

Differential Revision: [D25500303](https://our.internmc.facebook.com/intern/diff/D25500303)

[ghstack-poisoned]
SplitInfinity pushed a commit that referenced this pull request Dec 16, 2020
…modules with scriptable preparations (#45645)

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Differential Revision: D24039990

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

ghstack-source-id: 47a2a2c4e5f0a453ee9dd6aaa9637b6855985649
Pull Request resolved: #49242
…placing nn.modules with scriptable preparations (#45645)"


Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

Differential Revision: [D25500303](https://our.internmc.facebook.com/intern/diff/D25500303)

[ghstack-poisoned]
SplitInfinity pushed a commit that referenced this pull request Dec 16, 2020
…modules with scriptable preparations (#45645)

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Differential Revision: D24039990

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

ghstack-source-id: aaea5df1c826975075aa86fbd756ffe23aa161a9
Pull Request resolved: #49242
…placing nn.modules with scriptable preparations (#45645)"


Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

Differential Revision: [D25500303](https://our.internmc.facebook.com/intern/diff/D25500303)

[ghstack-poisoned]
SplitInfinity pushed a commit that referenced this pull request Jan 4, 2021
…modules with scriptable preparations (#45645)

Summary:
Fixes #45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: #42244

cc: zhangguanheng66

Reviewed By: dongreenberg, ngimel

Differential Revision: D24039990

Pulled By: zhangguanheng66

fbshipit-source-id: 4ddff2d353124af9c2ef22db037df7e3d26efe65

ghstack-source-id: 29f695f1731ffea0032364515c8f11fd2f0dc368
Pull Request resolved: #49242
@codecov
Copy link

codecov bot commented Jan 4, 2021

Codecov Report

Merging #49242 (4c9b07d) into gh/splitinfinity/82/base (b76822e) will increase coverage by 10.21%.
The diff coverage is 100.00%.

@@                      Coverage Diff                      @@
##           gh/splitinfinity/82/base   #49242       +/-   ##
=============================================================
+ Coverage                     70.47%   80.69%   +10.21%     
=============================================================
  Files                          1899     1899               
  Lines                        205922   205947       +25     
=============================================================
+ Hits                         145123   166179    +21056     
+ Misses                        60799    39768    -21031     

Copy link
Contributor

@dongreenberg dongreenberg left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your help with this!

@dongreenberg
Copy link
Contributor

@ppwwyyxx FYI

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in abe1fa4.

facebook-github-bot pushed a commit to facebookresearch/detectron2 that referenced this pull request Jan 8, 2021
Summary: Require pytorch/pytorch#49242

Reviewed By: theschnitz

Differential Revision: D25791139

fbshipit-source-id: 709c58782d52356c3da8e021bab59b929cc65198
@facebook-github-bot facebook-github-bot deleted the gh/splitinfinity/82/head branch January 9, 2021 15:17
hwangdeyu pushed a commit to hwangdeyu/pytorch that referenced this pull request Jan 14, 2021
…modules with scriptable preparations (pytorch#45645) (pytorch#49242)

Summary:
Pull Request resolved: pytorch#49242

Fixes pytorch#45072

As discussed with zdevito gchanan cpuhrsch and suo, this change allows developers to create custom preparations for their modules before scripting. This is done by adding a `__prepare_scriptable__` method to a module which returns the prepared scriptable module out-of-place. It does not expand the API surface for end users.

Prior art by jamesr66a: pytorch#42244

Test Plan: Imported from OSS

Reviewed By: dongreenberg

Differential Revision: D25500303

fbshipit-source-id: d3ec9005de27d8882fc29d02f0d08acd2a5c6b2c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed 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.

None yet

3 participants