Skip to content

Conversation

smessmer
Copy link
Contributor

@smessmer smessmer commented Nov 30, 2018

Stack:
    :black_circle:  #14642 Moved Backend to c10  💛
    :white_circle:  #14643 Fix include paths for Backend.h  💛
    :white_circle:  #14644 Move Layout to c10  💛
    :white_circle:  #14645 Fix include paths for Layout.h  💛
    :white_circle:  #14646 Move TensorOptions, DefaultTensorOptions and OptionsGuard to c10  💛
    :white_circle:  #14647 Fix include paths for TensorOptions, DefaultTensorOptions, OptionsGuard  💛
    :white_circle:  #14651 Remove TensorImpl -> LegacyTypeDispatch dependency  💛
    :white_circle:  #14658 Remove TensorImpl -> context_base dependency  💛

Unfortunately, TensorOptions depends on this, so we need it in c10.

Differential Revision: D13283495

Differential Revision: D13283495
Differential Version: 65035202
Differential Revision: D13283495
Differential Version: 65268356

namespace at {

namespace c10 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason this can't live with the actual implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's kind of common to keep it separate (boost does the same for optional I/O) because iostream is a huge header to pull in. Otherwise, there's no strong reason though and I don't have a strong opinion on it. It would definitely be more convenient to have it with the implementation.

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

with comments

zdevito pushed a commit to zdevito/ATen that referenced this pull request Dec 4, 2018
Summary:
Pull Request resolved: pytorch/pytorch#14642

Unfortunately, TensorOptions depends on this, so we need it in c10.

Reviewed By: ezyang

Differential Revision: D13283495

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants