Remove warnings pytest models

[ambujpawar]

• Closes Fix warnings in the tests #6155

[datumbox]

@ambujpawar Thanks for the PR.

The tests are failing and look related. Could you please have a look? Thanks!

[ambujpawar]

Thanks for the review. I tried to resolve those errors by just modifying the tests. But apparently, it was not possible for inceptionv3 model. Please try to have a look at the solution :)

[pmeier]

LGTM if CI is green. Thanks @ambujpawar! The current failing jobs

• binary_linux_conda_py3.9_cu116
• cmake_macos_cpu
• cmake_windows_cpu
• cmake_windows_gpu

are unrelated.

[ambujpawar]

Failing tests seems unrelated to the PR and I see that the related tasks are listed in issues

[pmeier]

The following could be related, but I'll leave that up to @datumbox since he knows the model tests better than I do.

_____________ test_detection_model[cuda-fasterrcnn_resnet50_fpn] ______________
Traceback (most recent call last):
  File "C:\Users\circleci\project\test\test_models.py", line 776, in check_out
    _assert_expected(output, model_name, prec=prec)
  File "C:\Users\circleci\project\test\test_models.py", line 117, in _assert_expected
    torch.testing.assert_close(output, expected, rtol=rtol, atol=atol, check_dtype=False, check_device=False)
  File "C:\Users\circleci\project\env\lib\site-packages\torch\testing\_comparison.py", line 1342, in assert_close
    assert_equal(
  File "C:\Users\circleci\project\env\lib\site-packages\torch\testing\_comparison.py", line 1093, in assert_equal
    raise error_metas[0].to_error(msg)
AssertionError: Tensor-likes are not close!

Mismatched elements: 61 / 80 (76.2%)
Greatest absolute difference: 182.7935028076172 at index (9, 1) (up to 0.01 allowed)
Greatest relative difference: inf at index (1, 0) (up to 0.01 allowed)

The failure occurred for item [0]['boxes']

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\circleci\project\test\test_models.py", line 804, in test_detection_model
    full_validation &= check_out(out)
  File "C:\Users\circleci\project\test\test_models.py", line 784, in check_out
    torch.testing.assert_close(
  File "C:\Users\circleci\project\env\lib\site-packages\torch\testing\_comparison.py", line 1342, in assert_close
    assert_equal(
  File "C:\Users\circleci\project\env\lib\site-packages\torch\testing\_comparison.py", line 1093, in assert_equal
    raise error_metas[0].to_error(msg)
AssertionError: Tensor-likes are not close!

Mismatched elements: 1 / 20 (5.0%)
Greatest absolute difference: 0.019545435905456543 at index (19,) (up to 0.01 allowed)
Greatest relative difference: 0.026511636142229424 at index (19,) (up to 0.01 allowed)

[ambujpawar]

I think it is unrelated, as it was happening over here #6589 before this PR was even submitted

[datumbox]

The highlighted failure on the fasterrcnn model is not related. The changes LGTM.

Only one question which we could resolve elsewhere.

[datumbox]

@pmeier Since you are the most Pythonic person I know, any idea why this works? We pass the inception_blocks keyword parameter before *args. We do exactly the same thing on QuantizableGoogLeNet. Could this because we dont actually have any positional args in the classes? I was expecting that the class would emit an error for putting positional arguments after named params but it doesn't.

[pmeier]

This is one of the Python quirks that are syntactically allowed, but should never be used due to its very weird behavior. TL;DR: foo(baz="baz", "bar") is not valid syntax while foo(baz="baz", *["bar"]) is. See python/cpython#82741.

I would advise to put the *args before the inception_blocks=... parameter to avoid confusion. Same for all other occurrences of this pattern. I don't know of a linter that checks this for us though.

[ambujpawar]

Done! 👍
LGTM now?

[pmeier]

Not sure if we want to do it in this PR or split this off to a follow-up cc @datumbox. Nevertheless this bothered me a bit to much TBH. I wrote a small flake8 plugin to find all occurrences of this. Put the following in a nuakw.py file in the root of the project

import ast
import contextlib


class NodeVisitor(ast.NodeVisitor):
    def __init__(self):
        self.errors = []

    def visit_Call(self, node: ast.Call):
        with contextlib.suppress(StopIteration):
            starred = next(starred for starred in node.args if isinstance(starred, ast.Starred))
            if any(
                (keyword.value.lineno, keyword.value.col_offset) < (starred.lineno, starred.col_offset)
                for keyword in node.keywords
            ):
                self.errors.append(
                    (node.lineno, node.col_offset, "NUAKW Argument unpacking after keyword argument", type(self))
                )

        self.generic_visit(node)


class NoUnpackingAfterKeyword:
    name = "no_unpacking_after_keyword"
    version = "0.0.1"

    def __init__(self, tree: ast.AST):
        self._tree = tree

    def run(self):
        visitor = NodeVisitor()
        visitor.visit(self._tree)
        yield from visitor.errors

Now, put the following at the bottom of setup.cfg

[flake8:local-plugins]
extension =
    NUAKW = nuakw:NoUnpackingAfterKeyword
paths =
    ./

Running flake8 torchvision yields the following errors:

torchvision/models/quantization/inception.py:44:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/inception.py:55:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/inception.py:66:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/inception.py:77:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/inception.py:88:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/inception.py:122:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/googlenet.py:42:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/googlenet.py:53:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/googlenet.py:77:9: NUAKW Argument unpacking after keyword argument
torchvision/models/quantization/mobilenetv3.py:86:9: NUAKW Argument unpacking after keyword argument

If we fix one, we should fix all.

[ambujpawar]

This seemed to be a short fix, so I have implement this in this PR itself :)

[ambujpawar]

cc @pmeier @datumbox Shall we go to master now?

[datumbox]

LGTM (again) @ambujpawar. Thank you very much for being ultra patient and flexible with us. You did end up fixing many more issues than originally described. I do apologise for this but I think your PR leaves our codebase in much better state than before.

@pmeier Thanks a lot for the explanation you gave on the thread above. I agree that the aforementioned syntax is problematic and should be avoided. We can review whether its avoidance should be done via linter or via our code reviews, offline to avoid further delays on this work.

I just kicked the CI once more, we should be able to merge this once everything looks green. :)

[github-actions]

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

[ambujpawar]

Thanks for the kind words. Looking forward to solving more issues
Thanks for yours patience.