-
Notifications
You must be signed in to change notification settings - Fork 80
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
Change default truss. #565
Conversation
@@ -29,8 +29,8 @@ | |||
DEFAULT_SPEC_VERSION = "2.0" | |||
DEFAULT_PREDICT_CONCURRENCY = 1 | |||
|
|||
DEFAULT_CPU = "500m" | |||
DEFAULT_MEMORY = "512Mi" | |||
DEFAULT_CPU = "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.
Changing these default will map to something that's not the supported lowest instance type (which is 1cpu, 2Gi memory minus daemonsets, which will need some buffer.
I think 750m, 1.5Gi should be safe but up to you.
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.
Ah gotcha. We wanted to make that change because the default values weren't super intuitive (500m? 512Mi?) but we weren't sure if Baseten required the buffer.
We should work on a way to make specifying resources more intuitive but it doesn't have to block this release.
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.
Changing these default will map to something that's not the supported lowest instance type (which is 1cpu, 2Gi memory minus daemonsets, which will need some buffer.
I tried deploying this and it did deploy on the 1x2 instance
@@ -1,19 +1,30 @@ | |||
from typing import Any | |||
""" | |||
This file is where the key logic for serving your model is defined. |
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 is a good help message, I'm just going to update it quick before merge with copy and links that match the docs
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.
cool -- go for 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.
The `Model` class is an interface between the ML model that you're packaging and the model server that you're running it on.
The main methods to implement here are:
* `load`: runs exactly once when the model server is spun up or patched and loads the model onto the model server. Include any logic for initializing your model, such as downloading model weights and loading the model into memory.
* `predict`: runs every time the model server is called. Include any logic for model inference and return the model output.
See https://truss.baseten.co/quickstart for more.
self._model = None | ||
|
||
def load(self): | ||
# Load model here and assign to self._model. |
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 actually like the assign to self._model help text here
Summary
Made the following changes
@philipkiely-baseten Getting rid of the external_packages is a little bit more of an involved change (requires a baseten deploy), so we can work towards that, but won't do that here.