Remove unnecessary whitespace in complex tensors by choidongyeon · Pull Request #36331 · pytorch/pytorch

choidongyeon · 2020-04-09T19:36:27Z

This PR addresses Issue #36279.
Previously, printing of complex tensors would sometimes yield extra spaces before the elements as shown below:

print(torch.tensor([[1 + 1.340j, 3 + 4j], [1.2 + 1.340j, 6.5 + 7j]], dtype=torch.complex64))

would yield

tensor([[(1.0000 + 1.3400j),
         (3.0000 + 4.0000j)],
        [(1.2000 + 1.3400j),
         (6.5000 + 7.0000j)]], dtype=torch.complex64)

This occurs primarily because when the max width for the element is being assigned, the formatter's max_width is calculated prior to truncating the float values. As a result, self.max_width would end up being much longer than the final length of the element string to be printed.

I address this by adding a boolean variable that checks if a complex tensor contains only ints and change the control flow for calculating self.max_width accordingly.

Here are some sample outputs of both float and complex tensors:

tensor([[0., 0.],
        [0., 0.]], dtype=torch.float64) 

tensor([[(0.+0.j), (0.+0.j)],
        [(0.+0.j), (0.+0.j)]], dtype=torch.complex64) 

tensor([1.2000, 1.3400], dtype=torch.float64) 

tensor([(1.2000+1.3400j)], dtype=torch.complex64) 

tensor([[(1.0000+1.3400j), (3.0000+4.0000j)],
        [(1.2000+1.3400j), (6.5000+7.0000j)]], dtype=torch.complex64) 

tensor([1.0000, 2.0000, 3.0000, 4.5000]) 

tensor([(1.+2.j)], dtype=torch.complex64)

cc @ezyang @anjali411 @dylanbespalko

dr-ci · 2020-04-09T19:37:33Z

💊 CircleCI build failures summary and remediations

As of commit 972eb13 (more details on the Dr. CI page):

1/2 failures introduced in this PR
1/2 broken upstream at merge base e311e53 since Apr 09
Please rebase on the viable/strict branch (expand for instructions)

If your commit is newer than viable/strict, you can try basing on an older, stable commit:
```
git fetch https://github.com/pytorch/pytorch viable/strict
git rebase --onto FETCH_HEAD $(git merge-base origin/master HEAD)
```
If your commit is older than viable/strict:
```
git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD
```
Check out the recency history of this "viable master" tracking branch.

XLA failure

Job pytorch_xla_linux_xenial_py3_6_clang7_test is failing. Please create an issue with title prefixed by [PT_BREAK] in pytorch/xla and link to to this PR. If you have questions, please reach out to @ailzhang / @dlibenzi / @JackCaoG.

🚧 1 upstream failure:

These were probably caused by upstream breakages:

pytorch_linux_xenial_py3_clang5_asan_test since Apr 09
- 🔁 rerun

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 21 times.

anjali411 · 2020-04-09T20:35:28Z

I just saw that this PR leads to this:

>>> torch.tensor([1+2j],dtype=torch.complex64)
tensor([(1 + 2.j)], dtype=torch.complex64)

It should instead output tensor([(1. + 2.j)], dtype=torch.complex64) So I think we should also fix this in this PR

anjali411 · 2020-04-09T20:38:02Z

also we should move this check inside format function within complex branch and not pass has_non_zero_decimal_val in format(...) since has_non_zero_decimal_val is only used by complex

choidongyeon · 2020-04-09T20:54:08Z

@anjali411 Thanks for pointing these out. Will work on them later today.

choidongyeon · 2020-04-09T21:05:19Z

I just saw that this PR leads to this:
>>> torch.tensor([1+2j],dtype=torch.complex64)
tensor([(1 + 2.j)], dtype=torch.complex64)
It should instead output tensor([(1. + 2.j)], dtype=torch.complex64) So I think we should also fix this in this PR

Addressed this in most recent commit. Will fix the second one in a bit.

choidongyeon · 2020-04-09T22:29:36Z

also we should move this check inside format function within complex branch and not pass has_non_zero_decimal_val in format(...) since has_non_zero_decimal_val is only used by complex

This was a great suggestion. The only problem is that format works with single element of a tensor rather than all elements of a tensor. Since we need to check the whole tensor, it wouldn't be possible to put the check inside of format. However, I was able to work with just the additions I had already made in the Formatter constructor for this PR. Makes the code much more DRY. Not sure how you feel about the attribute name complex_with_decimal though?

anjali411 · 2020-04-09T22:34:43Z

+                                                for value in tensor_view])
            for value in tensor_view:
-                value_str = '{}'.format(value)
+                if self.complex_dtype and self.complex_with_decimal:


if self.complex_dtype if self.complex_with_decimal: value_str = ('{{:.{}f}}').format(PRINT_OPTS.precision).format(value) else: value_str = "{:.0f}".format(value.item())

attribute name is okay though?

I think we should change it to has_non_zero_decimal_val as mentioned here :D

anjali411 · 2020-04-09T22:37:06Z

            tensor_view = tensor.reshape(-1)

        if not self.floating_dtype:
+            self.complex_with_decimal = False


I think we should change it to has_non_zero_decimal_val and perhaps add a comment that it's only used for complex

anjali411 · 2020-04-09T23:08:43Z

@choidongyeon looks good overall.
nit-

>>> import torch
>>> torch.tensor([1+2j])
tensor([(1. + 2.j)], dtype=torch.complex64)
>>> import numpy as np
>>> np.array([1+2j])
array([1.+2.j])

we should follow numpy and remove the extra space between real and imag values

choidongyeon · 2020-04-09T23:43:57Z

@choidongyeon looks good overall.
nit-
>>> import torch
>>> torch.tensor([1+2j])
tensor([(1. + 2.j)], dtype=torch.complex64)
>>> import numpy as np
>>> np.array([1+2j])
array([1.+2.j])
we should follow numpy and remove the extra space between real and imag values

Easy peasy. Updated the PR, also updated the PR summary with some sample current outputs.

anjali411

great job! thanks for working on this :D

facebook-github-bot

@anjali411 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

choidongyeon · 2020-04-09T23:55:52Z

@anjali411 Thanks for being so responsive!
Also, was there originally a comment about an unnecessary comma or am I making this up? I see an email but I don't see it in this thread..

anjali411 · 2020-04-09T23:57:47Z

@anjali411 Thanks for being so responsive!
Also, was there originally a comment about an unnecessary comma or am I making this up? I see an email but I don't see it in this thread..

of course! let me know if you'd like to work on other issues related to complex numbers.

yeah I realized it was something else and so I removed it

facebook-github-bot · 2020-04-10T03:51:10Z

@anjali411 merged this pull request in 2f5b523.

Summary: This PR addresses Issue pytorch#36279. Previously, printing of complex tensors would sometimes yield extra spaces before the elements as shown below: ``` print(torch.tensor([[1 + 1.340j, 3 + 4j], [1.2 + 1.340j, 6.5 + 7j]], dtype=torch.complex64)) ``` would yield ``` tensor([[(1.0000 + 1.3400j), (3.0000 + 4.0000j)], [(1.2000 + 1.3400j), (6.5000 + 7.0000j)]], dtype=torch.complex64) ``` This occurs primarily because when the max width for the element is being assigned, the formatter's max_width is calculated prior to truncating the float values. As a result, ```self.max_width``` would end up being much longer than the final length of the element string to be printed. I address this by adding a boolean variable that checks if a complex tensor contains only ints and change the control flow for calculating ```self.max_width``` accordingly. Here are some sample outputs of both float and complex tensors: ``` tensor([[0., 0.], [0., 0.]], dtype=torch.float64) tensor([[(0.+0.j), (0.+0.j)], [(0.+0.j), (0.+0.j)]], dtype=torch.complex64) tensor([1.2000, 1.3400], dtype=torch.float64) tensor([(1.2000+1.3400j)], dtype=torch.complex64) tensor([[(1.0000+1.3400j), (3.0000+4.0000j)], [(1.2000+1.3400j), (6.5000+7.0000j)]], dtype=torch.complex64) tensor([1.0000, 2.0000, 3.0000, 4.5000]) tensor([(1.+2.j)], dtype=torch.complex64) ``` cc ezyang anjali411 dylanbespalko Pull Request resolved: pytorch#36331 Differential Revision: D20955663 Pulled By: anjali411 fbshipit-source-id: c26a651eb5c9db6fcc315ad8d5c1bd9f4b4708f7

Remove unnecessary whitespace in complex tensors

3dfbfe7

Fix lint error

b2b3b1c

pytorchbot added the open source label Apr 9, 2020

Donna Choi added 2 commits April 9, 2020 12:52

Fix lint error

60a5a72

Fix lint error

36d67f3

anjali411 self-requested a review April 9, 2020 20:19

anjali411 added the module: complex Related to complex number support in PyTorch label Apr 9, 2020

Add missing . in complex tensor print

fff10b5

zhangguanheng66 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 9, 2020

Remove complex tensor decimal check from format

fc9a497

anjali411 reviewed Apr 9, 2020

View reviewed changes

Donna Choi added 2 commits April 9, 2020 15:44

Change attribute name to has_non_zero_decimal_val

f194b55

Remove stray comment

f6664ab

Remove space between real and imaginary parts of complex number

972eb13

anjali411 approved these changes Apr 9, 2020

View reviewed changes

facebook-github-bot reviewed Apr 9, 2020

View reviewed changes

facebook-github-bot closed this in 2f5b523 Apr 10, 2020

choidongyeon deleted the complex_whitespace branch April 10, 2020 02:54

facebook-github-bot added the merged label Apr 10, 2020

mruberry added the Merged label Oct 28, 2020

Conversation

choidongyeon commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci Bot commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CircleCI build failures summary and remediations

XLA failure

🚧 1 upstream failure:

Uh oh!

anjali411 commented Apr 9, 2020

Uh oh!

anjali411 commented Apr 9, 2020

Uh oh!

choidongyeon commented Apr 9, 2020

Uh oh!

choidongyeon commented Apr 9, 2020

Uh oh!

choidongyeon commented Apr 9, 2020

Uh oh!

anjali411 Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

choidongyeon Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

anjali411 Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anjali411 Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anjali411 commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choidongyeon commented Apr 9, 2020

Uh oh!

anjali411 left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

choidongyeon commented Apr 9, 2020

Uh oh!

anjali411 commented Apr 9, 2020

Uh oh!

facebook-github-bot commented Apr 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

choidongyeon commented Apr 9, 2020 •

edited

Loading

dr-ci Bot commented Apr 9, 2020 •

edited

Loading

anjali411 Apr 9, 2020 •

edited

Loading

anjali411 Apr 9, 2020 •

edited

Loading

anjali411 commented Apr 9, 2020 •

edited

Loading