Address printing inconsistency between float and complex tensors #35841

choidongyeon · 2020-04-01T22:08:13Z

See issue #33494 Complex number printing inconsistent with float.

Changes introduces an optional argument in Formatter's format function to discern whether a tensor is a float tensor or not. This way, there is consistency between float tensors and complex tensors so that the complex tensors print in the same manner as float tensors:

Only a decimal point and no zeros for integer values.
Trailing zeros only if the value is truly a float.
White space introduced to fill the gap so that +/- symbols and commas align.

Here are some example outputs.

print(torch.zeros((2,2), dtype=torch.float64))

yields

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

print(torch.zeros((2,2), dtype=torch.complex64))

previously yielded

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

and now yields

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

This new print version is more consistent with float tensor's pretty print.

The following example mixes integer and decimals:

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

This yields:

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

The following example

torch.tensor([1,2,3,4.5])

yields

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

dr-ci · 2020-04-01T22:18:36Z

💊 CircleCI build failures summary and remediations

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

💚 💚 Looks good so far! There are no CircleCI failures yet. 💚 💚

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 26 times.

choidongyeon · 2020-04-02T16:32:09Z

@anjali411, does this address the printing inconsistency to your liking?

anjali411 · 2020-04-06T15:32:02Z

torch/_tensor_str.py

            p = PRINT_OPTS.precision
-            ret = '({{:.{}f}} {{}} {{:.{}f}}j)'.format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
+            # format real and imaginary values according to type
+            real_val = '({{:.0f}}.    '.format(p).format(value.real) if value.real.is_integer() \


real_val = '({{:.0f}}.'.format(p).format(value.real) if value.real.is_integer() \

I think we should remove the extra white space

Addressed in PR update.

anjali411 · 2020-04-06T15:33:12Z

torch/_tensor_str.py

+            # format real and imaginary values according to type
+            real_val = '({{:.0f}}.    '.format(p).format(value.real) if value.real.is_integer() \
+                else '({{:.{}f}}'.format(p).format(value.real)
+            imag_val = '{{:.0f}}.j    )'.format(p).format(value.imag) if value.imag.is_integer() \


imag_val = '{{:.0f}}.j)'.format(p).format(value.imag) if value.imag.is_integer() \

same as above

Addressed in PR update.

anjali411 · 2020-04-06T15:37:51Z

I think this behavior is in line with what we do for floating point tensors:

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

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

for example, this: >>> torch.tensor([1,2,3,4.5])

gives

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

However the first case that you mentioned is correct and to generalize it I think we wouldn't want to print the zeros after decimal if none of the entries have any non zero value after decimal

choidongyeon · 2020-04-06T16:50:56Z

Updated the PR description with current outputs.

choidongyeon · 2020-04-06T19:42:46Z

Based on the CircleCI build failures summary and remediations, it seems that of the six failing tests, five of them are due to upstream breakages. My understanding based on other PRs I've read is that the sixth failure (rocmdeb) is flaky. Is there anything else I should address?

facebook-github-bot

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

anjali411 · 2020-04-07T14:55:32Z

torch/_tensor_str.py

            p = PRINT_OPTS.precision
-            ret = '({{:.{}f}} {{}} {{:.{}f}}j)'.format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
+            # format real and imaginary values according to type
+            if not is_float_tensor:


what are you trying to do here?

Strictly speaking, is_floating_point() returns false for complex, so it's misleading to have is_float_tensor check and do something for it under complex branch

You said we only want to truncate the zeros only in cases where all elements of the complex are integers. This checks to see if the complex tensor has any float components. I can change the variable name, but that's what it does.

hmm maybe change it to has_non_zero_decimal_val ?

Oh, I changed it to complex_contains_float. Can change it tohas_non_zero_decimal_val if you prefer that though, let me know!

yeah let's change it to has_non_zero_decimal_val :)

Okay! Doing it right now.

Updated PR, with values consistent as shown in the PR description.

anjali411 · 2020-04-08T16:40:50Z

Hey @choidongyeon thanks for the PR :D it looks good! wondering if you looked into why we have the extra space in printing, for eg.

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

is it because we are not using masked_select?

choidongyeon · 2020-04-08T17:27:12Z

@anjali411 Thanks for merging the changes from master in for me. Regarding your question about why we have the extra spaces - I haven't investigated too deeply but I know that it has to do with self.max_width. At t he moment, format returns: return (self.max_width - len(ret)) * ' ' + ret where self.max_width seems to be set in the Formatter's constructor. One way to address the issue would be to reverse the order of the formatting (assume integer, then filter out cases with actual decimals):

        elif self.complex_dtype:
            p = PRINT_OPTS.precision
            ret = "({{:.0f}} {{}} {{:.0f}}.j)".format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
            if has_non_zero_decimal_val:
                # complex tensor contains decimal values elements only
                return '({{:.{}f}} {{}} {{:.{}f}}j)'.format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
        else:
            ret = '{}'.format(value)
        return (self.max_width - len(ret)) * ' ' + ret

Then, our return values will look like this:

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])

The only thing is, I feel like there should be a more elegant solution to preserve the current form of the code (one return statement). I can create another issue and look into it separately. Or just push the proposed change here in. Personally, I think we should close this issue out by merging it, but let me know!

anjali411 · 2020-04-08T22:47:10Z

@anjali411 Thanks for merging the changes from master in for me. Regarding your question about why we have the extra spaces - I haven't investigated too deeply but I know that it has to do with self.max_width. At t he moment, format returns: return (self.max_width - len(ret)) * ' ' + ret where self.max_width seems to be set in the Formatter's constructor. One way to address the issue would be to reverse the order of the formatting (assume integer, then filter out cases with actual decimals):
        elif self.complex_dtype:
            p = PRINT_OPTS.precision
            ret = "({{:.0f}} {{}} {{:.0f}}.j)".format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
            if has_non_zero_decimal_val:
                # complex tensor contains decimal values elements only
                return '({{:.{}f}} {{}} {{:.{}f}}j)'.format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
        else:
            ret = '{}'.format(value)
        return (self.max_width - len(ret)) * ' ' + ret
Then, our return values will look like this:
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]) 
The only thing is, I feel like there should be a more elegant solution to preserve the current form of the code (one return statement). I can create another issue and look into it separately. Or just push the proposed change here in. Personally, I think we should close this issue out by merging it, but let me know!

Hi @choidongyeon I see. I agree we should figure out a more elegant solution for this. I'll merge this PR. would you like to create a follow up issue and work on it?

facebook-github-bot

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

choidongyeon · 2020-04-08T22:49:04Z

@anjali411 Thanks for merging the changes from master in for me. Regarding your question about why we have the extra spaces - I haven't investigated too deeply but I know that it has to do with self.max_width. At t he moment, format returns: return (self.max_width - len(ret)) * ' ' + ret where self.max_width seems to be set in the Formatter's constructor. One way to address the issue would be to reverse the order of the formatting (assume integer, then filter out cases with actual decimals):
        elif self.complex_dtype:
            p = PRINT_OPTS.precision
            ret = "({{:.0f}} {{}} {{:.0f}}.j)".format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
            if has_non_zero_decimal_val:
                # complex tensor contains decimal values elements only
                return '({{:.{}f}} {{}} {{:.{}f}}j)'.format(p, p).format(value.real, '+-'[value.imag < 0], abs(value.imag))
        else:
            ret = '{}'.format(value)
        return (self.max_width - len(ret)) * ' ' + ret
Then, our return values will look like this:
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]) 
The only thing is, I feel like there should be a more elegant solution to preserve the current form of the code (one return statement). I can create another issue and look into it separately. Or just push the proposed change here in. Personally, I think we should close this issue out by merging it, but let me know!
Hi @choidongyeon I see. I agree we should figure out a more elegant solution for this. I'll merge this PR. would you like to create a follow up issue and work on it?

Sounds like a plan! Will do that right now.

Edit: no idea if I did this correctly but here it is: #36279

facebook-github-bot · 2020-04-10T00:22:45Z

@anjali411 merged this pull request in fdf7a83.

…orch#35841) Summary: See issue [pytorch#33494 Complex number printing inconsistent with float](pytorch#33494). Changes introduces an optional argument in Formatter's ```format``` function to discern whether a tensor is a float tensor or not. This way, there is consistency between float tensors and complex tensors so that the complex tensors print in the same manner as float tensors: - Only a decimal point and no zeros for integer values. - Trailing zeros only if the value is truly a float. - White space introduced to fill the gap so that +/- symbols and commas align. Here are some example outputs. ``` print(torch.zeros((2,2), dtype=torch.float64)) ``` yields ``` tensor([[0., 0.], [0., 0.]], dtype=torch.float64) ``` ``` print(torch.zeros((2,2), dtype=torch.complex64)) ``` previously yielded ``` tensor([[(0.0000 + 0.0000j), (0.0000 + 0.0000j)], [(0.0000 + 0.0000j), (0.0000 + 0.0000j)]], dtype=torch.complex64) ``` and now yields ``` tensor([[(0 + 0.j), (0 + 0.j)], [(0 + 0.j), (0 + 0.j)]], dtype=torch.complex64) ``` This new print version is more consistent with float tensor's pretty print. The following example mixes integer and decimals: ``` print(torch.tensor([[1 + 1.340j, 3 + 4j], [1.2 + 1.340j, 6.5 + 7j]], dtype=torch.complex64)) ``` This yields: ``` tensor([[ (1.0000 + 1.3400j), (3.0000 + 4.0000j)], [ (1.2000 + 1.3400j), (6.5000 + 7.0000j)]], dtype=torch.complex64) ``` The following example ``` torch.tensor([1,2,3,4.5]) ``` yields ``` tensor([1.0000, 2.0000, 3.0000, 4.5000]) . ``` Pull Request resolved: pytorch#35841 Differential Revision: D20893848 Pulled By: anjali411 fbshipit-source-id: f84c533b8957a1563602439c07e60efbc79691bc

Donna Choi added 2 commits April 1, 2020 14:55

Fix float complex tensor print inconsistency

32a76df

Merge branch 'master' of github.com:pytorch/pytorch into complex_print

8276ff9

pytorchbot added the open source label Apr 1, 2020

choidongyeon changed the title ~~Complex print~~ Address printing inconsistency between float and complex tensors Apr 1, 2020

Address lint error

0efdb4a

zou3519 requested a review from anjali411 April 6, 2020 14:24

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

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

anjali411 reviewed Apr 6, 2020

View reviewed changes

Donna Choi added 2 commits April 6, 2020 09:08

Removed whitespace

961de38

Changes based on PR comments

a5ea4d0

choidongyeon requested a review from anjali411 April 6, 2020 20:25

facebook-github-bot reviewed Apr 7, 2020

View reviewed changes

anjali411 reviewed Apr 7, 2020

View reviewed changes

Change variable name

8beb5a0

choidongyeon requested a review from anjali411 April 7, 2020 19:03

Change argument variable name

65591a1

anjali411 approved these changes Apr 8, 2020

View reviewed changes

Merge https://github.com/pytorch/pytorch into complex_print

b83562a

facebook-github-bot reviewed Apr 8, 2020

View reviewed changes

choidongyeon mentioned this pull request Apr 8, 2020

Extra whitespace issue when printing complex tensors #36279

Closed

facebook-github-bot closed this in fdf7a83 Apr 9, 2020

choidongyeon deleted the complex_print branch April 9, 2020 17:54

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

mruberry added the Merged label Oct 28, 2020

Address printing inconsistency between float and complex tensors #35841

Address printing inconsistency between float and complex tensors #35841

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

💊 CircleCI build failures summary and remediations

Uh oh!

choidongyeon commented Apr 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

choidongyeon commented Apr 6, 2020

Uh oh!

choidongyeon commented Apr 6, 2020

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

anjali411 commented Apr 8, 2020

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

facebook-github-bot commented Apr 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

choidongyeon commented Apr 1, 2020 •

edited

Loading

dr-ci bot commented Apr 1, 2020 •

edited

Loading

anjali411 commented Apr 6, 2020 •

edited

Loading

anjali411 commented Apr 8, 2020 •

edited

Loading

choidongyeon commented Apr 8, 2020 •

edited

Loading

choidongyeon commented Apr 8, 2020 •

edited

Loading