Skip to content

Change default variants to 'function'.#11247

Closed
ezyang wants to merge 114 commits intomasterfrom
export-D9648570
Closed

Change default variants to 'function'.#11247
ezyang wants to merge 114 commits intomasterfrom
export-D9648570

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Sep 4, 2018 •
edited
Loading

Stack:
    :black_circle:  #11247 Change default variants to 'function'.  💚

Previously, the default for a declaration in native_functions.yaml
was ['function', 'method'], i.e., generate both a method and
function for every binding. We now believe this is inappropriate:
the majority of new kernels added to PyTorch should live as
free functions, NOT methods. Thus, we change the default accordingly.

I also took the opportunity to de-method some "internal" functions
that had a leading underscore. While, strictly speaking, this is a
BC breaking change, I believe it is highly unlikely anyone was using
these directly.

Differential Revision: D9648570

ezyang added 30 commits August 29, 2018 09:11
@ezyang
V1: Initial commit
0d445ca 
Differential Revision: D9557315
Differential Version: 56407314
@ezyang
V2: Delete from the constructor
f0d395a 
Differential Revision: D9557315
Differential Version: 56413294
@ezyang
V3: Kill a bunch of missing Context constructor
2bbf28c 
Differential Revision: D9557315
Differential Version: 56415719
@ezyang
V4: fix gesv code
c3fe926 
Differential Revision: D9557315
Differential Version: 56424143
@ezyang
V5: Merge with parent diff changes
e69e741 
Differential Revision: D9557315
Differential Version: 56437859
@ezyang
V2: Initial commit
7891e30 
Differential Revision: D9561478
Differential Version: 56437861
@ezyang
V3: s/Type/BaseType/ for parent call
4ed56e4 
Differential Revision: D9561478
Differential Version: 56440382
@ezyang
V4: Fix another linker error
cdee2c3 
Differential Revision: D9561478
Differential Version: 56440854
@ezyang
V1: Initial commit
851f3f5 
Differential Revision: D9562197
Differential Version: 56442349
@ezyang
V1: Initial commit
2144199 
Differential Revision: D9562312
Differential Version: 56443146
@ezyang
V2: Add an is_variable accessor
e624027 
Differential Revision: D9562312
Differential Version: 5644335
@ezyang
V1: Initial commit
7394abf 
Differential Revision: D9562467
Differential Version: 56443875
@ezyang
V2: Fix some include order problems; make backend() public
9add05a 
Differential Revision: D9562467
Differential Version: 56445436
@ezyang
V3: CR comments
4931693 
Differential Revision: D9562312
Differential Version: 56445886
@ezyang
V6: Commit rFBSb9862b8dda49194298bb1a4fbc214d466f3c8350
58eebe9 
Differential Revision: D9557315
Differential Version: 56444203
@ezyang
V5: Fix lint and mypy
f59ee58 
Differential Revision: D9561478
Differential Version: 56446440
@ezyang
V3: Correctly create temporary as necessary
d62c5c9 
Differential Revision: D9562467
Differential Version: 56447016
@ezyang
V4: Nab another options.type() site
79c57c3 
Differential Revision: D9562467
Differential Version: 56447216
@ezyang
V6: Fix clowntown
809e085 
Differential Revision: D9561478
Differential Version: 56449391
@ezyang
V1: Initial commit
b0f9c56 
Differential Revision: D9564206
Differential Version: 56452969
@ezyang
V4: TensorOptions temporary needed in codegen
26ba739 
Differential Revision: D9562312
Differential Version: 56453321
@ezyang
V1: Initial commit
3772dae 
Differential Revision: D9564516
Differential Version: 56455053
@ezyang
V5: Add missing AT_API
0f84d22 
Differential Revision: D9562467
Differential Version: 56473044
@ezyang
V2: (no description)
2ce21fa 
Differential Revision: D9562197
Differential Version: 56473286
@ezyang
V1: Initial commit
4c96822 
Differential Revision: D9578398
Differential Version: 56517363
@ezyang
V1: Initial commit
957de3c 
Differential Revision: D9578399
Differential Version: 56517362
@ezyang
V1: Initial commit
8373738 
Differential Revision: D9578734
Differential Version: 56519039
@ezyang
V2: Delete incomplete comment
4d8652c 
Differential Revision: D9578734
Differential Version: 56520207
@ezyang
V3: Don't use __builtin_unreachable
c8b500d 
Differential Revision: D9578734
Differential Version: 56526151
@ezyang
V2: Initial commit
0fec60e 
Differential Revision: D9581560
Differential Version: 56526196
@ezyang
V1: Initial commit
78bce12 
Differential Revision: D9646190
Differential Version: 56879138
@ezyang
V5: Merge with parent diff changes
ce6082d 
Differential Revision: D9631619
Differential Version: 56884928
@ezyang
V4: Merge with parent diff changes
b02341c 
Differential Revision: D9634750
Differential Version: 56884930
@ezyang
V2: Merge with parent diff changes
4dd52fd 
Differential Revision: D9634904
Differential Version: 56884926
@ezyang
V2: Merge with parent diff changes
cff9129 
Differential Revision: D9646190
Differential Version: 56884925
@ezyang
V3: (no description)
f6e6ab7 
Differential Revision: D9646190
Differential Version: 56896300
@ezyang
V1: Initial commit
7ca210e 
Differential Revision: D9648570
Differential Version: 56896711
@ezyang ezyang mentioned this pull request Sep 4, 2018
Closed
@ezyang
V3: Merge with parent diff changes
5112c8d 
Differential Revision: D9634904
Differential Version: 56940396
@ezyang
V4: Merge with parent diff changes
3b00b6a 
Differential Revision: D9646190
Differential Version: 56940395
@ezyang
V2: Merge with parent diff changes
d81b59f 
Differential Revision: D9648570
Differential Version: 56940398
@ezyang
V4: Fix braino
7daadd5 
Differential Revision: D9634904
Differential Version: 56941443
@ezyang
V5: Merge with parent diff changes
5f4ee06 
Differential Revision: D9646190
Differential Version: 56941456
@ezyang
V3: Merge with parent diff changes
ad4ab57 
Differential Revision: D9648570
Differential Version: 56941455
@ezyang
V4: Fix hphp, maybe
66121ae 
Differential Revision: D9648570
Differential Version: 56950051
@ezyang ezyang changed the base branch from export-D9646190 to master September 5, 2018 16:09
goldsborough
goldsborough approved these changes Sep 5, 2018
View reviewed changes
Copy link
Contributor

@goldsborough goldsborough left a comment

Choose a reason for hiding this comment

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

looks goat to me

@ezyang
V8: Rebase
dbd5749 
Differential Revision: D9648570
Differential Version: 57251564
cpuhrsch
cpuhrsch approved these changes Sep 7, 2018
View reviewed changes
Copy link
Contributor

@cpuhrsch cpuhrsch left a comment

Choose a reason for hiding this comment

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

cool

zdevito pushed a commit to zdevito/ATen that referenced this pull request Sep 8, 2018
@ezyang @facebook-github-bot
Change default variants to 'function'. (#11247)
216af30 
Summary:
Pull Request resolved: pytorch/pytorch#11247

Previously, the default for a declaration in native_functions.yaml
was ['function', 'method'], i.e., generate both a method and
function for every binding.  We now believe this is inappropriate:
the majority of new kernels added to PyTorch should live as
free functions, NOT methods.  Thus, we change the default accordingly.

I also took the opportunity to de-method some "internal" functions
that had a leading underscore.  While, strictly speaking, this is a
BC breaking change, I believe it is highly unlikely anyone was using
these directly.

Reviewed By: yf225

Differential Revision: D9648570

fbshipit-source-id: 8b94647b824e0899d6d18aa5585aaedc9d9957d2
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
@ezyang @PenghuiCheng
Change default variants to 'function'. (pytorch#11247)
f244c2e 
Summary:
Pull Request resolved: pytorch#11247

Previously, the default for a declaration in native_functions.yaml
was ['function', 'method'], i.e., generate both a method and
function for every binding.  We now believe this is inappropriate:
the majority of new kernels added to PyTorch should live as
free functions, NOT methods.  Thus, we change the default accordingly.

I also took the opportunity to de-method some "internal" functions
that had a leading underscore.  While, strictly speaking, this is a
BC breaking change, I believe it is highly unlikely anyone was using
these directly.

Reviewed By: yf225

Differential Revision: D9648570

fbshipit-source-id: 8b94647b824e0899d6d18aa5585aaedc9d9957d2
@soumith soumith deleted the export-D9648570 branch February 21, 2019 23:25
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuhrsch cpuhrsch cpuhrsch approved these changes

@apaszke apaszke Awaiting requested review from apaszke

@colesbury colesbury Awaiting requested review from colesbury

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

+1 more reviewer

@goldsborough goldsborough goldsborough approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ezyang @cpuhrsch @goldsborough