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

Add some named tensor helper functions #21636

Closed
wants to merge 3 commits into from

Conversation

zou3519
Copy link
Contributor

@zou3519 zou3519 commented Jun 11, 2019

Stack from ghstack:

at::internal_set_names_inplace(Tensor, optional)

  • Used for setting the names of a tensor. This isn't a native function for
    performance reasons; it'll be used to implement tensor.set_names_,
    torch.empty(..., names=), and other functions.

optional Tensor::names()

  • Access names of a tensor.

Test Plan:

  • New tests in [namedtensor ci]

gh-metadata: pytorch pytorch 21636 gh/zou3519/46/head

Differential Revision: D15780269

at::internal_set_names_inplace(Tensor, optional<DimnameList>)
- Used for setting the names of a tensor. This isn't a native function for
  performance reasons; it'll be used to implement tensor.set_names_,
  torch.empty(..., names=), and other functions.

optional<DimnameList> Tensor::names()
- Access names of a tensor.

Test Plan:
- New tests in [namedtensor ci]
@pytorchbot pytorchbot added the module: internals Related to internal abstractions in c10 and ATen label Jun 11, 2019
@zou3519 zou3519 requested a review from gchanan June 11, 2019 16:31
@gchanan gchanan requested a review from nairbv June 11, 2019 17:59
@gchanan
Copy link
Contributor

gchanan commented Jun 11, 2019

@nairbv can you take a look?

@@ -257,6 +267,7 @@ class CAFFE2_API Tensor {

/// Returns a `Tensor`'s dimension names data structure
NamedTensorMeta* get_named_tensor_meta() const;
NamedTensorMeta* get_named_tensor_meta();
Copy link
Collaborator

Choose a reason for hiding this comment

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

c++ question .... why don't these function signatures conflict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

c++ lets you overload member methods based on const-ness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Err I'm changing the signatures to const NamedTensorMeta* get_named_tensor_meta() const;. I am not sure if this is actually different.

Add some named tensor helper functions

at::internal_set_names_inplace(Tensor, optional<DimnameList>)
- Used for setting the names of a tensor. This isn't a native function for
  performance reasons; it'll be used to implement tensor.set_names_,
  torch.empty(..., names=), and other functions.

optional<DimnameList> Tensor::names()
- Access names of a tensor.

Test Plan:
- New tests in [namedtensor ci]

gh-metadata: pytorch pytorch 21636 gh/zou3519/46/head
Copy link
Collaborator

@nairbv nairbv left a comment

Choose a reason for hiding this comment

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

Tensor &

Add some named tensor helper functions

at::internal_set_names_inplace(Tensor, optional<DimnameList>)
- Used for setting the names of a tensor. This isn't a native function for
  performance reasons; it'll be used to implement tensor.set_names_,
  torch.empty(..., names=), and other functions.

optional<DimnameList> Tensor::names()
- Access names of a tensor.

Test Plan:
- New tests in [namedtensor ci]

gh-metadata: pytorch pytorch 21636 gh/zou3519/46/head
@zou3519
Copy link
Contributor Author

zou3519 commented Jun 11, 2019

@nairbv updated

@zou3519 zou3519 deleted the gh/zou3519/46/head branch June 12, 2019 19:37
zdevito pushed a commit to zdevito/ATen that referenced this pull request Jun 12, 2019
Summary:
Pull Request resolved: pytorch/pytorch#21636
ghimport-source-id: 5eff5744cd3c80f75bdb02576be1407a64e0434d

Differential Revision: D15780269

Pulled By: zou3519

fbshipit-source-id: 87ff40ffbe0ebd5fc4d105709c9f6f8dda5f9952
@facebook-github-bot
Copy link
Contributor

@zou3519 merged this pull request in 28adca8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged module: internals Related to internal abstractions in c10 and ATen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants