-
Notifications
You must be signed in to change notification settings - Fork 177
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
Fix failing FP6 benchmark #928
Fix failing FP6 benchmark #928
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/928
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 7fbbcca with merge base 653efe9 (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
thanks, this is using a higher level API, not old API. but it makes sense to keep consistent with test_ops I think |
actually, maybe just adding a comment explaining this is enough, it's easier for end users to use the high level |
probably better to just add a comment so it's easier for end users to use these benchmark script
Thanks for the feedback @jerryzh168. I don't think I made it clear enough, but currently the ao/benchmarks/benchmark_fp6.py Line 12 in 858205a
The |
oh I see, yeah makes sense to update this, it should be |
That makes sense -- let me update the script with your change, @jerryzh168. Do you think it makes sense to open a new PR and close the current one? |
I tested the change (it works) and created a new PR for it: #931. Closing this PR. |
Summary:
I was playing around with FP6 and noticed that its benchmark script is no longer functional. It seems that it's still using an older API. This PR fixes the script in accordance with the FP6 correctness test in
ao/test/test_ops.py
:ao/test/test_ops.py
Lines 62 to 70 in 653efe9
Here is the utility function
_create_floatx_inputs
:ao/test/test_ops.py
Lines 36 to 42 in 653efe9