[k8s] Fix GPU labelling nomenclature #3274
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Michaelvll
approved these changes
Mar 5, 2024
There was a problem hiding this comment.
Thanks for fixing this and aligning the GPU names @romilbhardwaj! LGTM with a small nit
| # 2. remove 'GeForce ' if present (e.g., 'NVIDIA GeForce RTX 3070' -> 'RTX 3070') | ||
| # 3. replace 'RTX ' with 'RTX' (without spaces) (e.g., 'RTX 6000' -> 'RTX6000') | ||
| # 4. replace any other spaces with dashes (e.g. 'RTX 2080 Ti' -> 'RTX2080-Ti') | ||
| gpu_name = gpu_name.lower().replace('nvidia ', '').replace('geforce ', '').replace('rtx ', 'rtx').replace(' ', '-') |
There was a problem hiding this comment.
Do we want to make the gpu name in the label uppercase to keep it align with the other parts?
There was a problem hiding this comment.
Generally kubernetes labels and values are lowercase, so keeping in line with convention. Also since label matching is case-sensitive, changing now would require deeper backward compatibility changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, a
RTX A6000gets labelled asA6000, which is a different card. This further causes confusion when the user tries to runsky launch --gpus a6000as prompted bysky local upoutput:with this PR, the naming for RTX series card is aligned with runpod, fluidstack and other clouds' catalogs.
RTXA6000RTX4090etc.
sky local up(which also invokes labelling) on A6000 and A100 machines from fluidstack.