Skip to content

Improve ET_CHECK_OR_RETURN_FALSE error messages in kernels/portable #9751

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

Merged
merged 1 commit into from
Mar 28, 2025

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Mar 28, 2025

We have a lot of ET_CHECK_OR_RETURN_FALSE that log a condition, but
not the values of the variables in that condition. This is an attempt
to improve debuggability of these errors.

cc @larryliu0820 @manuelcandales

[ghstack-poisoned]
@swolchok
Copy link
Contributor Author

swolchok commented Mar 28, 2025

Copy link

pytorch-bot bot commented Mar 28, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9751

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit d9b339b with merge base 65ebabb (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

swolchok added a commit that referenced this pull request Mar 28, 2025
We have a lot of `ET_CHECK_OR_RETURN_FALSE` that log a condition, but
not the values of the variables in that condition. This is an attempt
to improve debuggability of these errors.
ghstack-source-id: db6393ef8976772316b3ac99372ae3487beebeae
ghstack-comment-id: 2762245084
Pull-Request-resolved: #9751
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 28, 2025
@swolchok swolchok added module: kernels Issues related to kernel libraries and utilities, and code under kernels/ release notes: ops & kernels Changes to the opset and any new / changed kernel implementations and removed module: kernels Issues related to kernel libraries and utilities, and code under kernels/ labels Mar 28, 2025
@swolchok swolchok merged commit ebe8522 into main Mar 28, 2025
81 of 84 checks passed
@swolchok swolchok deleted the gh/swolchok/408/head branch March 28, 2025 22:06
swolchok added a commit that referenced this pull request Mar 31, 2025
ghstack-source-id: 2dcf8fe2d819bf730dbf18d0790d55f691754caa
ghstack-comment-id: 2766812039
Pull-Request-resolved: #9768

#9751 added a bunch of logging to error messages. We discovered after commit that it caused -Wformat warnings (turned into errors via -Werror) in ATen mode, because a bunch of Tensor methods return different types between ExecuTorch and ATen. This PR adds formatting macros to be used with this methods and uses them.
swolchok added a commit that referenced this pull request Mar 31, 2025
#9751 added a bunch of logging to error messages. We discovered after
commit that it caused -Wformat warnings (turned into errors via -Werror)
in ATen mode, because a bunch of Tensor methods return different types
between ExecuTorch and ATen. This PR adds formatting macros to be used
with this methods and uses them.
swolchok added a commit that referenced this pull request Apr 2, 2025
I forgot that we have a nice system for logging ScalarTypes as strings already. Let's use it.

ghstack-source-id: ed9f7fdd9ec36666837adf413718e35d9334b0fd
ghstack-comment-id: 2773731655
Pull Request resolved: #9845
swolchok added a commit that referenced this pull request Apr 3, 2025
I forgot that we have a nice system for logging ScalarTypes as strings
already. Let's use it.
kirklandsign pushed a commit that referenced this pull request Apr 11, 2025
…9751)

We have a lot of `ET_CHECK_OR_RETURN_FALSE` that log a condition, but
not the values of the variables in that condition. This is an attempt
to improve debuggability of these errors.

cc @larryliu0820 @manuelcandales
kirklandsign pushed a commit that referenced this pull request Apr 11, 2025
#9751 added a bunch of logging to error messages. We discovered after
commit that it caused -Wformat warnings (turned into errors via -Werror)
in ATen mode, because a bunch of Tensor methods return different types
between ExecuTorch and ATen. This PR adds formatting macros to be used
with this methods and uses them.
kirklandsign pushed a commit that referenced this pull request Apr 11, 2025
I forgot that we have a nice system for logging ScalarTypes as strings
already. Let's use it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: ops & kernels Changes to the opset and any new / changed kernel implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants