-
Notifications
You must be signed in to change notification settings - Fork 31.5k
[Whisper] Add rocm expected results to certain tests #40482
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
| cuda = [" Folks, I spend a lot of time right over there, night after night after night, actually. Carefully selecting for you the day's noosiest, most aerodynamic headlines, stress testing, and those topical anti-lock breaks and power steering, painstakingly stitching, leather seating so soft, it would make JD power and her associates blush to create the luxury sedan that is my nightly monologue. But sometimes, you sometimes, folks. I lurched a consciousness in the back of an abandoned school bus and slap myself awake."] | ||
| cuda1 = [" Folks, I spend a lot of time right over there, night after night after night, actually. Carefully selecting for you the day's noosiest, most aerodynamic headlines, stress testing, and those topical anti-lock breaks and power steering, painstakingly stitching, leather seating so soft, it would make JD power and her associates blush to create the luxury sedan that is my nightly monologue. But sometimes, you sometimes, folks. I lurched a consciousness in the back of an abandoned school bus and slap myself a wig."] | ||
| rocm = [" Folks, I spend a lot of time right over there, night after night after night, actually. Carefully selecting for you the day's noosiest, most aerodynamic headlines, stress testing, and those topical anti-lock breaks and power steering, painstakingly stitching, leather seating, so soft, it would make JD power and her associates blush to create the luxury sedan that is my nightly monologue. But sometimes, you sometimes, folks, I lurched a consciousness in the back of an abandoned school bus and slap myself awake."] | ||
| # fmt: on | ||
| expected_output = Expectations({("cuda", None): cuda, ("rocm", None): rocm}).get_expectation() | ||
| expected_output1 = Expectations({("cuda", None): cuda1, ("rocm", None): rocm}).get_expectation() |
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.
This one is interesting. Notice how cuda and cuda1 end with different words.
This did not happen when I ran it on the MI325. If the second result is supposed to end with "wig" then we have to look into it.
@eustlb thoughts?
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.
It seems like cuda output uses this generation config while cuda1 uses a different generation config so they are expected to have different results.
So I think with rocm you should generate another output with the same config as cuda1, to have something like rocm1.
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.
I think they could generate different results because the generation config are not the same, yet it's possible that the two results be the same. Even with cuda and cuda1 the two generations are very close, so it's not a stretch to think that on rocm the output tokens are the same for the two generations.
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.
Right, but the purpose of the test is to verify that the generation config has the expected effect.
So we either have a rocm bug or need to adjust the config.
|
run-slow: whisper |
|
This comment contains run-slow, running the specified jobs: models: ['models/whisper'] |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
remi-or
left a comment
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.
LGTM just add precise versions for ROCm please!
| cuda1 = [" Folks, I spend a lot of time right over there, night after night after night, actually. Carefully selecting for you the day's noosiest, most aerodynamic headlines, stress testing, and those topical anti-lock breaks and power steering, painstakingly stitching, leather seating so soft, it would make JD power and her associates blush to create the luxury sedan that is my nightly monologue. But sometimes, you sometimes, folks. I lurched a consciousness in the back of an abandoned school bus and slap myself a wig."] | ||
| rocm = [" Folks, I spend a lot of time right over there, night after night after night, actually. Carefully selecting for you the day's noosiest, most aerodynamic headlines, stress testing, and those topical anti-lock breaks and power steering, painstakingly stitching, leather seating, so soft, it would make JD power and her associates blush to create the luxury sedan that is my nightly monologue. But sometimes, you sometimes, folks, I lurched a consciousness in the back of an abandoned school bus and slap myself awake."] | ||
| # fmt: on | ||
| expected_output = Expectations({("cuda", None): cuda, ("rocm", None): rocm}).get_expectation() |
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.
Specify version for rocm
ahadnagy
left a comment
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.
Thanks Ivar! i think it's fine not specifying the exact platform versions until we need to differentiate.
|
run-slow: whisper |
|
This comment contains run-slow, running the specified jobs: models: ['models/whisper'] |
remi-or
left a comment
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.
approved w/ one very minor nit
|
[For maintainers] Suggested jobs to run (before merge) run-slow: whisper |
What does this PR do?
Adds expected results to certain tests that had slightly different results on AMD devices.