-
Notifications
You must be signed in to change notification settings - Fork 143
Make exceptions less verbose by default #1330
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
base: main
Are you sure you want to change the base?
Conversation
21f4232
to
9e376ef
Compare
The |
9e376ef
to
ff479ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR simplifies PyTensor exception messages by reducing their verbosity by default, making it easier for users to find the actual error message. The changes introduce a new "low" verbosity level that shows minimal information with a hint to increase verbosity for more details.
- Adds a new "low" exception verbosity level as the default
- Simplifies error message formatting in function calls
- Provides hints to users about how to get more detailed error information
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
pytensor/link/utils.py | Adds hint message for low verbosity exceptions |
pytensor/configdefaults.py | Updates exception verbosity config to include new "medium" level and simplified description |
pytensor/compile/function/types.py | Refactors exception handling to use cleaner messages and respect verbosity settings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ff479ac
to
8002918
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
8002918
to
f575025
Compare
f575025
to
11376ac
Compare
Failing jax test addressed in #1646 |
How do errors in numba mode look after this PR? |
if config.exception_verbosity == "low" | ||
else get_variable_trace_string(self.maker.inputs[i].variable) | ||
) | ||
e.add_note(f"\nInvalid {argument_name} to {function_name} at index {i}.{where}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we grab the name of the symbolic input and show that as well (if its not None)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
One thing that's still frustrating is that the traceback is not useful at all. Adding the information in the error message about which input is causing the error is great, but seeing the line |
I approved but I actually think the runtime error is a step backwards. You cut away the part of the traceback that points to the actionable python code, this one:
|
I disagree. First, 100% of PyTensor users are not generating their own functions, pymc and other frameworks are. You define an RV and then you get an obscure error because some join inputs created in the Model.logp_dlogp_function has a weird value in an operation you never called (the ones in the density). Second, this example is short on purpose. In practice you have a traceback that is 100 lines long. Users just give up before finding the actual error raised (SpecifyShape failed) Third, the info on how to get more details is right there. Change the config flag. If you need (which most times you don't, it's either immediately obviously or you would never be able to act on it) you can. Myself I would always prefer the 2 step workflow. I already know the error, now let's find where it's coming from. The order / relevance of info is flipped in the old approach, the way it was presented. |
Numba compile errors are completely unreadable but that's pretty much out of our control. Runtime errors like the SpecifyShape should look better, I can get an example output |
TypeError: Wrong number of dimensions: expected 2, got 1 with shape (1,). It's pretty obvious? The function that raises it is obscure, but you should read tracebacks end to top, so you start close to the useful info. This PR tries to make it easier to locate the useful info by removing "helpful" cruft that was being appended. We shouldn't hide the natural Python traceback though. That would be a big anti-pattern. |
Here is a motivating example: https://discourse.pymc.io/t/debug-mode-in-pytensor/14348/12?u=ricardov94 Note the user defined line (the one they're responsible for) shows up not at the end but 4-5 stacks before. Before that there's a lot of text, appended after the actual error. In an ideal world you would show the line the user wrote and the final error (in that order). But we can't know what's the "line the user wrote". That example is still more direct than what you would get in logp evals as I mentioned above but it's already terrible enough that the user had no clue where to start |
PyTensor exceptions are very verbose by default. Users often struggle to even find the actual error message.
This PR makes exceptions more minimal with a hint to set the relevant flag
exception_verbosity
tomedium
(the oldlow
orhigh
) for more details.The type error in the following snippet:
Now looks like:
Whereas with the old default looked like
A runtime error:
Now looks like this
Whereas before it looked like
📚 Documentation preview 📚: https://pytensor--1330.org.readthedocs.build/en/1330/