-
Notifications
You must be signed in to change notification settings - Fork 25.6k
enable more ASAN tests #101483
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
enable more ASAN tests #101483
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/101483
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit e380233: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
9a75dcf
to
eb23ae4
Compare
95eff87
to
b18de17
Compare
@pytorchbot label "topic: not user facing" |
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.
Looks like your changes to build.sh
make build-asan.sh
redundant. If that's the case, please delete it.
Also, please include in the PR description how much this increases runtime of the tests.
And how one makes sure that the same test would not run on multiple shards(which is always the danger with .cpp tests) By the way, @huydhn haven't you automated running cpp tests and sharing them, that renders all those conditions in test.sh useless?
If not, than perhaps most prudent thing to do would be to create separate shard for C++ tests
Let's gather the information about the increase in test time and create a separate shard if need. At the moment, running C++ tests via run_test.py runs them in parallel (but still locally within one runner as I haven't setup the part to gather test time to enable sharding across runners yet) |
f4c96c7
to
66cc8f4
Compare
A misaligned error in PackedEmbeddingBagWeight::prepack may be relevant. |
b39c610
to
3e38e3f
Compare
f1ae1f9
to
9b1b5a2
Compare
23d28d4
to
729c6cf
Compare
@huydhn The slowest ASAN test takes over 2 hours. |
The increase was about 20m in the slowest shard. So I think we could tolerate that. A follow-up task would be to re-balance these ASAN shards. There are 6 of them and the 1st and 2nd shards need to handle more load atm. |
@pytorchmergebot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Recently, we are seeing some bugs found by ASAN such as #101400, I think enabling ASAN for more tests is necessary to catch more hidden bugs.
cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10