-
Notifications
You must be signed in to change notification settings - Fork 104
Fix the cuda benchmark parameter value in the examples #129
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
elezar
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
| metadata: | ||
| namespace: sharing-demo | ||
| name: sharing-demo-job | ||
| name: sharing-demo-job-1 |
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 suppose one question I have is whether we should also rename the quickstart pod?
Also, does this require a readme update?
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 file was never meant to be run. It was just for showing during the demo at Kubecon NA 2023. The name of the job is the same because in the demo I flip screens and show how the spec got expanded.
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.
There's no such an example in the quickstart folder. The README in mig+msps doesn't mention sharing-demo-job-1.yaml. Perhaps we can add a description later.
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 file is only here because it's what I started off showing here:
https://youtu.be/1QfShSQLsbs?si=9rFmRUMi-e_puwO2&t=1128
Its not in the README because by the time I get to the README in the demo I don't need this file anymore.
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.
If you find it confusing to have in here but never meant to be run, I'd just remove it rather than update it.
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 could cause confusion to users. I've removed the file.
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.
Is the folder name mig+mps misleading too? There's a mig+ts configuration in the example. How about renaming the folder to mig?
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.
The purpose is to demonstrate mig+mps, the other configs are for comparison.
Signed-off-by: Yuan Chen <[email protected]> Remove job-1 Signed-off-by: Yuan Chen <[email protected]>
63ce239 to
96210bb
Compare
|
This demo only works with classic DRA and will be removed soon. |
number of bodiesin the cuda benchmark should be a multiple of 256. Replace the existing values of4226000with4226048in the examples.mig+mps/sharing-demo/sharing-demo-job-1.yaml, which is not used.