Skip to content

Add WITH_LOCATION macros for propagating external library errors. #9441

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

Closed

Conversation

ysiraichi
Copy link
Collaborator

This PR enhances PyTorch/XLA's error handling and status management, specifically for interactions with external libraries. In summary, this PR:

  • Introcudes XLA_RETURN_IF_ERROR_WITH_LOCATION and XLA_ASSIGN_OR_RETURN_WITH_LOCATION: new macros designed to propagate statuses from external library calls, including the exact source code location where the call to the library function that returned an error.
  • Adds test coverage: new test cases have been added to validate the behavior of these location-specific macro variants.

These updates should improve our ability to track and debug issues originating from external library.

@ysiraichi

This comment was marked as outdated.

@ysiraichi ysiraichi force-pushed the ysiraichi/add-status-propagation-for-external-libs branch from 918f90a to 540d62e Compare July 2, 2025 18:24
@ysiraichi ysiraichi force-pushed the ysiraichi/fix-status-location-logic branch from 0427c0a to d6ba41f Compare July 2, 2025 18:27
@ysiraichi ysiraichi force-pushed the ysiraichi/add-status-propagation-for-external-libs branch from 540d62e to 487988e Compare July 2, 2025 18:28
@ysiraichi ysiraichi force-pushed the ysiraichi/fix-status-location-logic branch 2 times, most recently from 0242960 to 620e4b6 Compare July 10, 2025 12:06
@ysiraichi ysiraichi force-pushed the ysiraichi/add-status-propagation-for-external-libs branch from 487988e to f072112 Compare July 10, 2025 18:51
@ysiraichi ysiraichi marked this pull request as ready for review July 10, 2025 18:52
@ysiraichi ysiraichi changed the base branch from ysiraichi/fix-status-location-logic to master July 10, 2025 18:52
- Add `XLA_RETURN_IF_ERROR_WITH_LOCATION` macro for external library status propagation
- Add `XLA_ASSIGN_OR_RETURN_WITH_LOCATION` macro for external library status handling
- Enhance test coverage with new test cases for location-specific macro variants
- Improve macro documentation to clarify internal vs external usage patterns
@zhanyong-wan
Copy link
Collaborator

Hi Yukio,

Let's discuss the design of this.

I'm not sure that it's worthwhile asking the programmer to make the decision on whether they should collect the source location at each callsite. It's tedious. Also, the programmer may decide wrong and make it hard to see the source location at run time.

I think we should just always collect the location, but don't show it by default. If the user requests to see the C++ context (BTW, I just found out about https://docs.pytorch.org/docs/stable/debugging_environment_variables.html - we should use the TORCH_SHOW_CPP_STACKTRACES env var to control the behavior), we will show the source locations.

Does this make sense? Thanks.

@ysiraichi
Copy link
Collaborator Author

Got it. So, I will close this PR, and follow-up with what we discussed in our meeting.

@ysiraichi ysiraichi closed this Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants