Deprecate old GPU classes #2848
Conversation
erikbern
force-pushed
the
erikbern/deprecate-gpu-classes
branch
from
b48122e to
acadd87
Compare
There was a problem hiding this comment.
One thing that we use the GPU classes for is to document the details of each GPU in the docstring for the relevant class so that it shows up here: https://modal.com/docs/reference/modal.gpu
I think we need to move most of this reference page to a "guide" page so that the information is preserved (and can be updated independently of the client in the future).
I would also add some notes on the deprecation of the GPU classes to this page, i.e. in the docstring to the modal.gpu module.
modal/gpu.py
Outdated
| return api_pb2.GPUConfig() | ||
| else: | ||
| raise InvalidError(f"Invalid GPU config: {value}. Value must be a string, a `GPUConfig` object, or `None`.") | ||
| raise InvalidError( | ||
| f"Invalid GPU config: {value}. Value must be a string or `None` (or a deprecated `_GPUConfig` object)." |
There was a problem hiding this comment.
Why are we pointing users towards the private version of the object?
There was a problem hiding this comment.
_GPUConfig is just the base class of all the public objects
There was a problem hiding this comment.
Yeah just seems confusing to mention it in a public error message. Maybe "or a modal.gpu object, although these are deprecated"?
There was a problem hiding this comment.
sure sgtm, i can update
I actually did this last week. We used to link to the reference from the guide when listing all the GPU types, but the guide now lists all the GPU types instead: https://modal.com/docs/guide/gpu (although we should probably expand this). I think it makes more sense to have an exhaustive list of GPU types in the guide and not the reference.
Sure can do |
|
Oh nice I missed that. I do see a couple uses of the |
We don't use GPUs in integration tests but I have some separate PRs to update a few things that was using |
modal/gpu.py
Outdated
| return api_pb2.GPUConfig() | ||
| else: | ||
| raise InvalidError(f"Invalid GPU config: {value}. Value must be a string, a `GPUConfig` object, or `None`.") | ||
| raise InvalidError( | ||
| f"Invalid GPU config: {value}. Value must be a string or `None` (or a deprecated `_GPUConfig` object)." |
There was a problem hiding this comment.
Yeah just seems confusing to mention it in a public error message. Maybe "or a modal.gpu object, although these are deprecated"?
erikbern
force-pushed
the
erikbern/deprecate-gpu-classes
branch
from
c687b14 to
c02cf66
Compare
|
Thanks @mwaskom! |
Going forward the only sanctioned way to configure GPU is through strings. This is nice because it's a lot less code.
Changelog
gpu=A100(...)etc) in favor of just using strings (gpu="A100"etc)