KEMBAR78
Fix Python 3.8 expecttest machinery again, this time for good. by ezyang · Pull Request #60044 · pytorch/pytorch · GitHub
Skip to content

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Jun 15, 2021

Stack from ghstack:

In #59709 I attempted to fix the expecttest machinery to work in Python
3.8. However, I noticed that it would fail to do substitutions in this
case:

    self.assertExpectedInline(
        foo(),
        """bar"""
    )

This is because the triple quoted string is not on the same line as the
backtrace line number (at the very beginning), and for safety reasons
the preexisting regex refused to search beyond the first line. This
wasn't a big deal prior to Python 3.8 because the flipped version of
the regex simply required the triple quoted string to be flush with
the end of the statement (which it typically was!) But it is a big deal
now that we only have the start of the statement.

I couldn't think of a way to fix this in the current model, so I decided
to call in the big guns. Instead of trying to do the regex with only
the start xor end line number, I now require you provide BOTH line numbers,
and we will only regex within this range. The way we compute these line
numbers is by parsing the Python test file with ast, and then searching
through statements until we find one that is consistent with the line
number reported by the backtrace. If we don't find anything, we
conservatively assume that the string lies exactly in the backtrace
(and you'll probably fail the substitution in that case.)

The resulting code is quite a lot simpler (no more reversed regex) and
hopefully more robust, although I suppose we are going to have to do
some field testing.

Signed-off-by: Edward Z. Yang ezyang@fb.com

Differential Revision: D29146943

In #59709 I attempted to fix the expecttest machinery to work in Python
3.8.  However, I noticed that it would fail to do substitutions in this
case:

```
    self.assertExpectedInline(
        foo(),
        """bar"""
    )
```

This is because the triple quoted string is not on the same line as the
backtrace line number (at the very beginning), and for safety reasons
the preexisting regex refused to search beyond the first line.  This
wasn't a big deal prior to Python 3.8 because the flipped version of
the regex simply required the triple quoted string to be flush with
the end of the statement (which it typically was!)  But it is a big deal
now that we only have the start of the statement.

I couldn't think of a way to fix this in the current model, so I decided
to call in the big guns.  Instead of trying to do the regex with only
the start xor end line number, I now require you provide BOTH line numbers,
and we will only regex within this range.  The way we compute these line
numbers is by parsing the Python test file with ast, and then searching
through statements until we find one that is consistent with the line
number reported by the backtrace.  If we don't find anything, we
conservatively assume that the string lies exactly in the backtrace
(and you'll probably fail the substitution in that case.)

The resulting code is quite a lot simpler (no more reversed regex) and
hopefully more robust, although I suppose we are going to have to do
some field testing.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 15, 2021

💊 CI failures summary and remediations

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


  • 3/3 failures possibly* introduced in this PR
    • 1/3 non-scanned failure(s)

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_xenial_cuda11_1_cudnn8_py3_gcc7_test2 (1/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jun 16 20:18:54 ERROR [66.735s]: test_rref_to_h...n__.TensorPipeTensorPipeAgentCudaRpcTestWithSpawn)
Jun 16 20:18:54     actual = rref.remote().forward(rref_x, True).to_here()
Jun 16 20:18:54   File "/opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/rref_proxy.py", line 18, in _invoke_rpc
Jun 16 20:18:54     rref_type = rref._get_type(timeout=timeout)
Jun 16 20:18:54   File "/opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/api.py", line 409, in _rref_typeof_on_user
Jun 16 20:18:54     return fut.wait()
Jun 16 20:18:54 RuntimeError: RPCErr:1:RPC ran for more than set timeout (60000 ms) and will now be marked with an error
Jun 16 20:18:54 
Jun 16 20:18:54 
Jun 16 20:18:54 
Jun 16 20:18:54 ======================================================================
Jun 16 20:18:54 ERROR [66.735s]: test_rref_to_here_synchronization4 (__main__.TensorPipeTensorPipeAgentCudaRpcTestWithSpawn)
Jun 16 20:18:54 ----------------------------------------------------------------------
Jun 16 20:18:54 Traceback (most recent call last):
Jun 16 20:18:54   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_distributed.py", line 398, in wrapper
Jun 16 20:18:54     self._join_processes(fn)
Jun 16 20:18:54   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_distributed.py", line 590, in _join_processes
Jun 16 20:18:54     self._check_return_codes(elapsed_time)
Jun 16 20:18:54   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_distributed.py", line 633, in _check_return_codes
Jun 16 20:18:54     raise RuntimeError(error)
Jun 16 20:18:54 RuntimeError: Process 1 exited with error code 10 and exception:
Jun 16 20:18:54 Traceback (most recent call last):

See CircleCI build pytorch_linux_xenial_py3_6_gcc5_4_test (2/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jun 16 19:47:56 AssertionError: False is not tr...lowed difference with rtol=0 and atol=0 is only 0!
Jun 16 19:47:56   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_distributed.py", line 516, in run_test
Jun 16 19:47:56     getattr(self, test_name)()
Jun 16 19:47:56   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_distributed.py", line 400, in wrapper
Jun 16 19:47:56     fn()
Jun 16 19:47:56   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/distributed/distributed_test.py", line 4591, in test_ddp_logging_data_cpu
Jun 16 19:47:56     self.assertEqual(ddp_logging_data.get("find_unused_parameters"), 0)
Jun 16 19:47:56   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_utils.py", line 1419, in assertEqual
Jun 16 19:47:56     super().assertTrue(result, msg=self._get_assert_msg(msg, debug_msg=debug_msg))
Jun 16 19:47:56   File "/opt/conda/lib/python3.6/unittest/case.py", line 682, in assertTrue
Jun 16 19:47:56     raise self.failureException(msg)
Jun 16 19:47:56 AssertionError: False is not true : Scalars failed to compare as equal! Comparing 1 and 0 gives a difference of 1, but the allowed difference with rtol=0 and atol=0 is only 0!
Jun 16 19:47:56 
Jun 16 19:47:56 
Jun 16 19:47:56 
Jun 16 19:47:56 ----------------------------------------------------------------------
Jun 16 19:47:56 Ran 220 tests in 59.371s
Jun 16 19:47:56 
Jun 16 19:47:56 FAILED (errors=1, skipped=110)
Jun 16 19:47:56 
Jun 16 19:47:56 Generating XML reports...
Jun 16 19:47:56 Generated XML report: test-reports/dist-gloo/distributed.test_distributed_fork/TEST-TestDistBackendWithFork-20210616194657.xml

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 to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@ezyang ezyang requested review from bdhirsh and walterddr June 15, 2021 21:57
@ezyang
Copy link
Contributor Author

ezyang commented Jun 15, 2021

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

…ood."

In #59709 I attempted to fix the expecttest machinery to work in Python
3.8.  However, I noticed that it would fail to do substitutions in this
case:

```
    self.assertExpectedInline(
        foo(),
        """bar"""
    )
```

This is because the triple quoted string is not on the same line as the
backtrace line number (at the very beginning), and for safety reasons
the preexisting regex refused to search beyond the first line.  This
wasn't a big deal prior to Python 3.8 because the flipped version of
the regex simply required the triple quoted string to be flush with
the end of the statement (which it typically was!)  But it is a big deal
now that we only have the start of the statement.

I couldn't think of a way to fix this in the current model, so I decided
to call in the big guns.  Instead of trying to do the regex with only
the start xor end line number, I now require you provide BOTH line numbers,
and we will only regex within this range.  The way we compute these line
numbers is by parsing the Python test file with ast, and then searching
through statements until we find one that is consistent with the line
number reported by the backtrace.  If we don't find anything, we
conservatively assume that the string lies exactly in the backtrace
(and you'll probably fail the substitution in that case.)

The resulting code is quite a lot simpler (no more reversed regex) and
hopefully more robust, although I suppose we are going to have to do
some field testing.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
if isinstance(n, ast.Expr):
if hasattr(n, 'end_lineno'):
assert LINENO_AT_START
end_lineno = n.end_lineno # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

plz fix quickcheck

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i saw it

…ood."

In #59709 I attempted to fix the expecttest machinery to work in Python
3.8.  However, I noticed that it would fail to do substitutions in this
case:

```
    self.assertExpectedInline(
        foo(),
        """bar"""
    )
```

This is because the triple quoted string is not on the same line as the
backtrace line number (at the very beginning), and for safety reasons
the preexisting regex refused to search beyond the first line.  This
wasn't a big deal prior to Python 3.8 because the flipped version of
the regex simply required the triple quoted string to be flush with
the end of the statement (which it typically was!)  But it is a big deal
now that we only have the start of the statement.

I couldn't think of a way to fix this in the current model, so I decided
to call in the big guns.  Instead of trying to do the regex with only
the start xor end line number, I now require you provide BOTH line numbers,
and we will only regex within this range.  The way we compute these line
numbers is by parsing the Python test file with ast, and then searching
through statements until we find one that is consistent with the line
number reported by the backtrace.  If we don't find anything, we
conservatively assume that the string lies exactly in the backtrace
(and you'll probably fail the substitution in that case.)

The resulting code is quite a lot simpler (no more reversed regex) and
hopefully more robust, although I suppose we are going to have to do
some field testing.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
@ezyang
Copy link
Contributor Author

ezyang commented Jun 16, 2021

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

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in 0bf1260.

@facebook-github-bot facebook-github-bot deleted the gh/ezyang/1039/head branch June 20, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants