Skip to content
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

[better_errors] Consolidate TracingDebugInfo and JaxprDebugInfo #26001

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gnecula
Copy link
Collaborator

@gnecula gnecula commented Jan 21, 2025

Previously, JaxprDebugInfo was a copy-and-paste of most of
TracingDebugInfo. Now we make it a proper extension.

@gnecula gnecula self-assigned this Jan 21, 2025
@gnecula gnecula added the pull ready Ready for copybara import and testing label Jan 21, 2025
Most places in the code assumed this already, but often
that usage is error reporting code, which is not yet well tested.

When we cannot get the `inspect.Signature` or when the
args and kwargs do not match the signature, we generate
the flattened argument names as: `args[0]`, `args[1]`,
`kwargs['foo']`, ... Previously, in these cases we
returned `arg_names` is None, and then the whole
debug_info ended up being `None`, throwing away even
available information.

We also add support for `api_util.fun_sourceinfo` even
for cases when the `fun.__code__` is not available. In
those cases we used to say that `fun_sourceinfo` is
`None`. Now, we use the string representation of `fun`
to get the name of built-in functions, or we use "<unknown>".
Previously, `JaxprDebugInfo` was a copy-and-paste of most of
`TracingDebugInfo`. Now we make it a proper extension to make
the relationship clearer.
@gnecula
Copy link
Collaborator Author

gnecula commented Jan 21, 2025

I am not convinced this refactoring is worth it, too much boilerplate. I'll put this on hold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant