cache traceback together with exceptions#6748
Merged
pmeier merged 1 commit intopytorch:mainfrom Oct 12, 2022
Merged
Conversation
datumbox
approved these changes
Oct 12, 2022
Contributor
datumbox
left a comment
There was a problem hiding this comment.
Kind of ugly but we already use a similar logic for caching the output... I'm approving as I agree that seeing the actual exceptions is necessary for debugging (I was left quite a few times scratching my head to find out what went wrong on tests) but I'm hoping we can adopt a better solution soon.
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 17, 2022
Reviewed By: NicolasHug Differential Revision: D40427479 fbshipit-source-id: 8783039e680fc74328ddd8d8ccf3fe39658345a2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We are caching the scripting of kernels inside our prototype tests:
vision/test/test_prototype_transforms_functional.py
Lines 21 to 22 in 11a2eed
We do this because the test
vision/test/test_prototype_transforms_functional.py
Line 83 in 11a2eed
is run multiple times for the same kernel, but with different sample inputs. In fact, we only have 78
KernelInfo's but the test is run 7826 times. Meaning, in 99% of the runs, we script the kernel without any information gain whatsoever.However, with the current implementation of
@cache, the traceback of re-raised cached exceptions is not cleaned properly bypytest. TL;DR: the traceback of an exception is mutable. By re-raising, thepytestframes are getting appended over and over again, butpytestonly removes them once. For a detailed explanation see pytest-dev/pytest#10363.This leads to ginormous tracebacks that drown the actual information in the noise. For example, I intentionally made
pad_bounding_boxnon-scriptable:pytest -q test/test_prototype_transforms_functional.py::TestKernels::test_scripted_vs_eager -k pad_bounding_box | wc -lmain: 499510mainwithout@cache: 3358The few extra lines stem from the fact that the
@cachewrapper adds minimal traceback itself.