-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Use the correct CSV filenames for MPS benchmark #142034
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/142034
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 4a85860 with merge base 86f306b ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @huydhn, thanks for putting this up. Yeah the only useful stat here will be the absolute runtime, since MacOS currently only supports this benchmark in eager mode. The idea is that we'll use this to track performance regressions over time, but individual runs on their own won't tell us much. I'll discuss with Nikita & Kulin, but we may want to format this dashboard a little differently than the inductor data, so that there's no confusion. Either way it's great to see the data showing up on the dashboard, from my end I'll follow up with a PR to enable the remaining workloads |
|
@pytorchbot merge -f "Lint is green" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This goes together with pytorch/pytorch#142034. MPS benchmark is running in eager mode at the moment.
After pytorch#135386 and pytorch#141999, MPS benchmark has been running for a while and the data has been uploaded correctly. However, the dashboard is still using the old schema that requires the output CSV files to be named in a certain way for its query to work https://github.com/pytorch/test-infra/blob/main/torchci/clickhouse_queries/compilers_benchmark_performance/query.sql#L32-L40. Specifically, the filename needs to be in the following format `inductor_${backend}_${suite}_${dtype}_${mode}_${device}_${target}.csv`. The new schema gets away with all this hacky setup, but the dashboard hasn't been migrated to the new schema yet. So, this is a quick way to just get the data to show up first. ### Testing https://github.com/pytorch/pytorch/actions/runs/12153886764 Pull Request resolved: pytorch#142034 Approved by: https://github.com/skotapati, https://github.com/malfet
After #135386 and #141999, MPS benchmark has been running for a while and the data has been uploaded correctly. However, the dashboard is still using the old schema that requires the output CSV files to be named in a certain way for its query to work https://github.com/pytorch/test-infra/blob/main/torchci/clickhouse_queries/compilers_benchmark_performance/query.sql#L32-L40. Specifically, the filename needs to be in the following format
inductor_${backend}_${suite}_${dtype}_${mode}_${device}_${target}.csv.The new schema gets away with all this hacky setup, but the dashboard hasn't been migrated to the new schema yet. So, this is a quick way to just get the data to show up first.
Testing
https://github.com/pytorch/pytorch/actions/runs/12153886764