Skip to content

Get rid of type() method on TensorOptions; use at::getType instead #11023

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

Get rid of type() method on TensorOptions; use at::getType instead #11023

wants to merge 10 commits into from

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Aug 29, 2018 •
edited
Loading

Get rid of type() method on TensorOptions; use at::getType instead

I'd like TensorOptions to not know anything about Context, so I can
move it to ATen/core without pulling in Context. To do this, the
type() method has to go, since it consults the context to get a Type.

Differential Revision: D9562467

Stacked on #11145

@ezyang
V1: Initial commit
7394abf 
Differential Revision: D9562467
Differential Version: 56443875
@ezyang ezyang changed the title Get rid of type() method on TensorOptions; use at::getType instead [WIP] Get rid of type() method on TensorOptions; use at::getType instead Aug 29, 2018
@ezyang
Copy link
Contributor Author

ezyang commented Aug 29, 2018

(This doesn't compile yet)

goldsborough
goldsborough reviewed Aug 29, 2018
View reviewed changes
aten/src/ATen/native/TensorFactories.cpp Outdated
function_name,
" is not implemented for sparse types, got: ",
options.type().toString());
at::getType(options).toString());

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/native/TensorFactories.cpp Outdated

Tensor eye(int64_t n, int64_t m, const TensorOptions& options) {
auto tensor = options.type().tensor({});
auto tensor = at::getType(options).tensor({});

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@ezyang
V2: Fix some include order problems; make backend() public
9add05a 
Differential Revision: D9562467
Differential Version: 56445436
@ezyang ezyang changed the title [WIP] Get rid of type() method on TensorOptions; use at::getType instead Get rid of type() method on TensorOptions; use at::getType instead Aug 29, 2018
ezyang added 2 commits August 29, 2018 14:14
@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 ezyang mentioned this pull request Aug 29, 2018
Closed
@ezyang
Copy link
Contributor Author

ezyang commented Aug 29, 2018

@pytorchbot retest this please

1 similar comment
@ezyang
Copy link
Contributor Author

ezyang commented Aug 30, 2018

@pytorchbot retest this please

@ezyang
V5: Add missing AT_API
0f84d22 
Differential Revision: D9562467
Differential Version: 56473044
gchanan
gchanan reviewed Aug 30, 2018
View reviewed changes
aten/src/ATen/TensorOptions.h Outdated
}
DeviceGuard guard(options.device());
return options.type().copy(tensor, non_blocking);
return at::getType(options).copy(tensor, non_blocking);

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

cpuhrsch
cpuhrsch reviewed Aug 30, 2018
View reviewed changes
aten/src/ATen/native/TensorFactories.cpp Outdated
function_name,
" expects floating point dtypes, got: ",
options.type().toString());
at::getType(options).toString());

This comment was marked as off-topic.

Sign in to view

cpuhrsch
cpuhrsch approved these changes Aug 30, 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.

accepted, but please work through comments first

ezyang added 2 commits August 30, 2018 20:58
@ezyang
V6: Merge with parent diff changes
74e329b 
Differential Revision: D9562467
Differential Version: 56608519
@ezyang
V7: Rebased
6cac035 
Differential Revision: D9562467
Differential Version: 56650790
@ezyang ezyang changed the base branch from export-D9562312 to export-D9613897 August 31, 2018 15:36
ezyang added 3 commits August 31, 2018 08:46
@ezyang
V8: rebase
630f2c4 
Differential Revision: D9562467
Differential Version: 56651350
@ezyang
V9: Handle CR comments on factories
9638802 
Differential Revision: D9562467
Differential Version: 56652506
@ezyang
V10: Merge with parent diff changes
8b6eb46 
Differential Revision: D9562467
Differential Version: 56672440
zdevito pushed a commit to zdevito/ATen that referenced this pull request Aug 31, 2018
@ezyang @facebook-github-bot
Get rid of type() method on TensorOptions; use at::getType instead (#…
a0940ff 
…11023)

Summary:
Pull Request resolved: pytorch/pytorch#11023

I'd like TensorOptions to not know anything about Context, so I can
move it to ATen/core without pulling in Context.  To do this, the
type() method has to go, since it consults the context to get a Type.

Reviewed By: cpuhrsch

Differential Revision: D9562467

fbshipit-source-id: 61a18a76eb042a5e70b64b963501e9d68c25d4f0
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
@ezyang @PenghuiCheng
Get rid of type() method on TensorOptions; use at::getType instead (p…
4db22ce 
…ytorch#11023)

Summary:
Pull Request resolved: pytorch#11023

I'd like TensorOptions to not know anything about Context, so I can
move it to ATen/core without pulling in Context.  To do this, the
type() method has to go, since it consults the context to get a Type.

Reviewed By: cpuhrsch

Differential Revision: D9562467

fbshipit-source-id: 61a18a76eb042a5e70b64b963501e9d68c25d4f0
@soumith soumith deleted the export-D9562467 branch February 21, 2019 12:09
@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

@gchanan gchanan gchanan left review comments

@cpuhrsch cpuhrsch cpuhrsch approved these changes

@apaszke apaszke Awaiting requested review from apaszke

@colesbury colesbury Awaiting requested review from colesbury

@ebetica ebetica Awaiting requested review from ebetica

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

+1 more reviewer

@goldsborough goldsborough goldsborough left review comments

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.

4 participants

@ezyang @cpuhrsch @gchanan @goldsborough