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

Failing to Format with an Ignored Matrix Matrix #1562

Closed
Queuecumber opened this issue Jul 26, 2020 · 3 comments
Closed

Failing to Format with an Ignored Matrix Matrix #1562

Queuecumber opened this issue Jul 26, 2020 · 3 comments
Labels
T: bug Something isn't working

Comments

@Queuecumber
Copy link

Queuecumber commented Jul 26, 2020

Describe the bug

I get INTERNAL ERROR: Black produced code that is not equivalent to the source when I try to format my code which contains a matrix that I have manually formatted and told black to ignore using # fmt: off, the full source code is:

import torch
from torch import Tensor

def to_ycbcr(x: Tensor) -> Tensor:
    r"""
    Converts a Tensor from RGB color space to YCbCr color space

    Parameters
    ----------
    x : Tensor
        The input Tensor holding an RGB image in [0, 255]. Can be in :math:`(N, C, H ,W)` or :math:`(C, H, W)` format.

    Returns
    -------
    Tensor
        The YCbCr result in [0, 255] of the same shape as the input.

    Note
    -----
    This function implements the "full range" conversion used by JPEG, e.g. it does **not** implement the ITU-R BT.601 standard which 
    many libraries (excluding PIL) use as the default definition of YCbCr. This conversion is given by:

    .. math::
        \begin{aligned}
        Y&=&0&+(0.299&\cdot R)&+(0.587&\cdot G)&+(0.114&\cdot B) \\
        C_{B}&=&128&-(0.168736&\cdot R)&-(0.331264&\cdot G)&+(0.5&\cdot B) \\
        C_{R}&=&128&+(0.5&\cdot R)&-(0.418688&\cdot G)&-(0.081312&\cdot B)
        \end{aligned}
    
    """
    # fmt: off
    ycbcr_from_rgb = torch.Tensor([
        0.29900, 0.58700, 0.11400,
        -0.168735892, -0.331264108, 0.50000,
        0.50000, -0.418687589, -0.081312411
    ]).view(3, 3).transpose(0, 1)

    b = torch.Tensor([0, 128, 128]).view(1, 3, 1, 1)

    if x.is_cuda:
        ycbcr_from_rgb = ycbcr_from_rgb.cuda()
        b = b.cuda()

    x = torch.einsum('cv,bcxy->bvxy', [ycbcr_from_rgb, x])
    x += b

    return x.contiguous()


def to_rgb(x: Tensor) -> Tensor:
    r"""
    Converts a Tensor from YCbCr color space to RGB color space

    Parameters
    ----------
    x : Tensor
        The input Tensor holding a YCbCr image in [0, 255]. Can be in :math:`(N, C, H ,W)` or :math:`(C, H, W)` format.

    Returns
    -------
    Tensor
        The RGB result in [0, 255] of the same shape as the input.

    Note
    -----
    This function expects the input to be "full range" conversion used by JPEG, e.g. it does **not** implement the ITU-R BT.601 standard which 
    many libraries (excluding PIL) use as the default definition of YCbCr. If the input came from this library or from PIL it should be fine.
    The conversion is given by:

    .. math::
        \begin{aligned}
        R&=&Y&&&+1.402&\cdot (C_{R}-128) \\
        G&=&Y&-0.344136&\cdot (C_{B}-128)&-0.714136&\cdot (C_{R}-128 ) \\
        B&=&Y&+1.772&\cdot (C_{B}-128)&
        \end{aligned}
    
    """
    # fmt: off
    rgb_from_ycbcr = torch.Tensor([
        1, 0, 1.40200,
        1, -0.344136286, -0.714136286,
        1, 1.77200, 0
    ]).view(3, 3).transpose(0, 1)

    b = torch.Tensor([0, 128, 128]).view(1, 3, 1, 1)

    if x.is_cuda:
        rgb_from_ycbcr = rgb_from_ycbcr.cuda()
        b = b.cuda()

    x -= b
    x = torch.einsum('cv,bcxy->bvxy', [rgb_from_ycbcr, x])

    return x.contiguous()

The two matrices ycbcr_from_rgb and rgb_from_ycbcr seem to be causing the problem, if I remove # fmt: off then it runs to completion but of course destroys the formatting on the matrices.

When I do this on the black playground, which seems to still have an issue but without a visible error message (e.g. it just gives back incorrect code) I get a weird extra indentation on the second function, almost like black is interpreting it to be part of the first function. I think this can link may work.

To Reproduce Steps to reproduce the behavior:

  1. Take this file '...' See above
  2. Run Black on it with these arguments '....' No combination of arguments resolves the problem
  3. See error

Expected behavior Formats my code correctly

Environment (please complete the following information):

  • Version: master
  • OS and Python version: Linux/Python 3.7.5

Does this bug also happen on master?

Yes, verified on the playground and on my local machine.

@Queuecumber Queuecumber added the T: bug Something isn't working label Jul 26, 2020
@Queuecumber
Copy link
Author

Queuecumber commented Jul 26, 2020

This seems to happen any time I have a matrix like this that I want ignored for formatting, I also have one like this:

    # fmt: off
    zigzag_indices = Tensor([
         0,  1,  5,  6, 14, 15, 27, 28,
         2,  4,  7, 13, 16, 26, 29, 42,
         3,  8, 12, 17, 25, 30, 41, 43,
         9, 11, 18, 24, 31, 40, 44, 53,
        10, 19, 23, 32, 38, 45, 52, 54,
        20, 22, 33, 38, 46, 51, 55, 60,
        21, 34, 36, 47, 50, 56, 59, 61,
        35, 36, 48, 49, 57, 58, 62, 63 
    ]).long()

that causes that file to fail, however that file is much longer so I only posted the shorter one.

Interestingly, I have several matrices that are defined outside of any function and those seem to work fine.

@ichard26
Copy link
Collaborator

ichard26 commented Jul 26, 2020

The bug where Black misbehaves when a # fmt: off comments applies outside of a function body is filed as #569. To avoid this bug, I recommend that you turn on formatting right after the thing you don't want Black to touch. Beware that formatting control comments pairs should be on the stay indention level or Black will break. I assume that you only want Black not to touch your quite lovely matrices, so I would put a # fmt: on comment right after the matrix.

     # fmt: off
     zigzag_indices = Tensor([
          0,  1,  5,  6, 14, 15, 27, 28,
          2,  4,  7, 13, 16, 26, 29, 42,
          3,  8, 12, 17, 25, 30, 41, 43,
          9, 11, 18, 24, 31, 40, 44, 53,
         10, 19, 23, 32, 38, 45, 52, 54,
         20, 22, 33, 38, 46, 51, 55, 60,
         21, 34, 36, 47, 50, 56, 59, 61,
         35, 36, 48, 49, 57, 58, 62, 63 
     ]).long()
+    # fmt: on

And if I do this to the matrices in your file, Black doesn't error out anymore:

(black) richard-26@ubuntu-laptop:~/programming/black$ diff -u --color original.py test.py 
--- original.py	2020-07-26 17:13:45.733409772 -0400
+++ test.py	2020-07-26 17:14:23.469736395 -0400
@@ -34,6 +34,7 @@
         -0.168735892, -0.331264108, 0.50000,
         0.50000, -0.418687589, -0.081312411
     ]).view(3, 3).transpose(0, 1)
+    # fmt: on
 
     b = torch.Tensor([0, 128, 128]).view(1, 3, 1, 1)
 
@@ -81,6 +82,7 @@
         1, -0.344136286, -0.714136286,
         1, 1.77200, 0
     ]).view(3, 3).transpose(0, 1)
+    # fmt: on
 
     b = torch.Tensor([0, 128, 128]).view(1, 3, 1, 1)
 
(black) richard-26@ubuntu-laptop:~/programming/black$ black test.py
reformatted test.py
All done! ✨ 🍰 ✨
1 file reformatted.

Hopefully that helps! Thanks for the bug report.


Environment:

  • OS Version: Ubuntu 18.04.4 LTS
  • Python version: CPython 3.8.1
  • Black version: master (2c5041c)

@Queuecumber
Copy link
Author

Queuecumber commented Jul 26, 2020

That did fix it, for some reason I thought I already tried that

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants