Report NotFound when a CUDA delegate's weights blob is missing - #22311
Open
shoumikhin wants to merge 1 commit into
Open
Report NotFound when a CUDA delegate's weights blob is missing#22311shoumikhin wants to merge 1 commit into
shoumikhin wants to merge 1 commit into
Conversation
The CUDA backend moves a model's constants into a separate weights blob and binds them while loading the method. When that blob is not supplied, the load path logged at Info level and returned Ok, so the model finished initialising with its constant pointers still null. The program then failed much later, inside a generated kernel, as an illegal memory access at a null base plus a field offset, reported as InvalidArgument from execute(). The cached load path already returns NotFound for the same condition, so this makes the other path agree. A missing blob is harmless when a model has no constants to bind, so the check keys on the constant count rather than on the blob being absent. get_num_constants is an optional symbol; when it is not available the two cases cannot be told apart, so the previous permissive behaviour is kept.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22311
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit f94f8ad with merge base c27baa8 ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CUDA backend moves a model's constants into a separate weights blob and binds them while loading the method. When that blob is not supplied, the load path logged a message at Info level and returned Ok, so the model finished initialising with its constant pointers still null. The program then failed much later, inside a generated kernel, as an illegal memory access:
That address is a null base plus a field offset, and the reported error was
0x12(InvalidArgument) fromexecute(), which points nowhere near the real cause.The cached load path already returns
NotFoundfor the same condition, so this makes the other path agree.A missing blob is genuinely harmless when a model has no constants to bind, and that case is common, so the check keys on the constant count rather than on the blob being absent.
get_num_constantsis an optional symbol; when it is not available the two cases cannot be told apart, so the previous permissive behaviour is kept.Before, on a model with three buffers whose blob was not supplied:
After:
0x20isError::NotFound.Test plan
Added
backends/cuda/tests/test_missing_weights_blob.py. It lowers a module with three buffers, checks the externalized blob holds real data rather than being an empty placeholder, then loads the program without it and requires the load to fail.Verified both directions against a build of this branch:
1 failed,AssertionError: RuntimeError not raised, and the log shows initialisation completing (container handle and CUDA stream both created)1 passedBuilt and ran on an aarch64 CUDA machine with GCC 13.3.
clang-formatreports no changes for the modified file, andufmtreports the new test already formatted.