- Notifications
You must be signed in to change notification settings - Fork 149
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 |
Ok this is all convincing. Having the flag is helpful. I recognize the point that pytensor is meant to be a low-level language for developers, and an end user often doesn't even know she's using it, so the tracebacks can't always find "the line". That said, on this:
No, it's not obvious at all. But if we have the additional flags to go deeper, that's great. The more information/hints we can give about where the actual error is in the computational graph, the better. I think at high levels of error verbosity, it would be great to show the dprint of the graph with a But that's 100% out of scope for this PR, which I agree is a step forward from our current awful tracebacks. Also, on this:
The problem is that we're not necessarily even in python code when the error is raised. Every runtime error will be triggered on a call to |
| The error is about the input to the function, it's always python land. It expected a matrix but received a vector. I don't know how we can be any more clear. TypeError: Wrong number of dimensions: expected 2, got 1 with shape (1,). Invalid argument to PyTensor function at index 0.I'll add the name if available |
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_verbositytomedium(the oldloworhigh) 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/