Skip to content

Conversation

sethlee0111
Copy link
Contributor

@sethlee0111 sethlee0111 commented Jul 10, 2023

Stack from ghstack (oldest at bottom):

This is the first diff to support logging of raw tensors for TensorBoard Intermediate Logging

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

  1. add tensor_proto() to summary.py which takes a tensor and convert to protobuf
  2. add add_tensor() to writer.py
  3. formatting changes introduced by arc lint

Differential Revision: D47302017

This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------

ghstack-source-id: 194155757

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

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Jul 10, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/104908

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit 88e722f:

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

],
proto_val_field: torch.view_as_real(tensor).tolist()
if proto_val_field == "scomplex_val" or proto_val_field == "dcomplex_val"
else tensor.reshape(-1).tolist(),
Copy link
Contributor

@vadimkantorov vadimkantorov Jul 10, 2023

Choose a reason for hiding this comment

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

here a bit more idiomatic and modern might be .flatten() instead of reshape(-1)

also, the two branches are currently not equivalent. if multidim tensors can be accepted here, the view_as_real(...).tolist() can produce nested lists

Also beware that tensor.reshape(-1).tolist() can produce python scalar instead of the list for numel() == 1 inputs: #52262 - my proposal for that one is to introduce a tolist(force = True) which would always return a list, even for scalars (so .numel() == 1 and .numel() == 0 probably should be checked)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added checks for numel() == 1 and numel() == 0

This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
sethlee0111 added a commit that referenced this pull request Jul 11, 2023
Pull Request resolved: #104908

This diff applies changes to open-source PyTorch to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------

ghstack-source-id: 194300671
ghstack-source-id: 194300671

Differential Revision: [D47357048](https://our.internmc.facebook.com/intern/diff/D47357048/)
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
)

# type maps: torch.Tensor type -> (protobuf type, protobuf val field)
type_map = {
Copy link

Choose a reason for hiding this comment

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

make this a private constant map outside the function? _TENSOR_TYPE_MAP = ... or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, applied changes.

Copy link

@kunalb kunalb left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, thanks for adding tests! some nits inline


if tensor.dtype in type_map:
proto_val_field = type_map[tensor.dtype][1]
proto_val_contents = (
Copy link

Choose a reason for hiding this comment

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

For readability, I'd still this make this explicit
proto_val_contents = []
if proto_val_field == ...:
proto_val_contents = ...
elif tensor.numel() == 1
proto_val_contents = ...

and so on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean. Made changes.

This is the first diff to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------


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

[ghstack-poisoned]
sethlee0111 added a commit that referenced this pull request Jul 12, 2023
Pull Request resolved: #104908

This diff applies changes to open-source PyTorch to support logging of raw tensors for [TensorBoard Intermediate Logging](https://www.internalfb.com/intern/wiki/TensorBoard/Intermediate_Logging/)

Ultimately, we aim to support the feature where store full tensor is stored as a tensor protobuf to TB. Protobuf contains shape, dtype, and elements of the given tensor.

1. add `tensor_proto()` to `summary.py` which takes a tensor and convert to protobuf
2. add `add_tensor()` to `writer.py`
3. formatting changes introduced by `arc lint`
-------------

ghstack-source-id: 194385501
ghstack-source-id: 194385501

Differential Revision: [D47357048](https://our.internmc.facebook.com/intern/diff/D47357048/)
@facebook-github-bot
Copy link
Contributor

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 13, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@atalman
Copy link
Contributor

atalman commented Jul 13, 2023

@pytorchbot revert -c ignoredsignal -m "broke win-vs2019-cpu-py3 and macos-12-py3-arm64 "

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

Can't revert PR that was landed via phabricator as D47302017. Please revert by going to the internal diff and clicking Unland.

@facebook-github-bot
Copy link
Contributor

@pytorchbot revert -m="Diff reverted internally" -c="ghfirst"

This Pull Request has been reverted by a revert inside Meta. To re-land this change, please open another pull request, assign the same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk).)

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

@sethlee0111 your PR has been successfully reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged Reverted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants