Various fixes to enable tempo to pack environment for single models #189
Conversation
sakoush
changed the title
| @@ -6,6 +6,8 @@ | |||
| from tempo.serve.metadata import ModelDataArgs, ModelDetails | |||
| from tempo.serve.protocol import Protocol | |||
|
|
|||
| _REQUEST_NUMPY_CONTENT_TYPE = {"content_type": "np"} | |||
There was a problem hiding this comment.
Is this a MLServer specific parameter. The KFServing V2 could be for any compliant V2 server like Triton. Would need to check if this will be ignored by them?
There was a problem hiding this comment.
yes I think this is specific to MLServer. Doing a quick search on "content_type" in the triton repo doesnt result in anything that suggests that it is being used. We need to check then that it will be ignored by triton as you suggested.
this bit is a little bit convoluted I admit, so we might want to think how to do it properly.
There was a problem hiding this comment.
The parameters field of the v2 payload should accept any arbitrary field. So unless Triton already uses this field for something else, I think that it should be alright.
| @@ -8,7 +10,13 @@ | |||
| DefaultRemoteFilename = "remote.pickle" | |||
| DefaultEnvFilename = "environment.tar.gz" | |||
|
|
|||
| MLServerEnvDeps = ["mlserver==0.4.0"] | |||
| MLServerEnvDeps = ["mlserver==0.4.1.dev1"] | |||
There was a problem hiding this comment.
What are the changes in MLServer to require the dev1 release?
There was a problem hiding this comment.
SeldonIO/MLServer#273 and SeldonIO/MLServer#286
These are specific changes required for being able to serve pytorch models (via mlflow) and it needed numpy fixes.
There was a problem hiding this comment.
Now that the parallel inference fix for the outlier example is also in, should we build a 0.4.2 release and use that instead?
|
|
||
|
|
||
| def _get_env(conda_env_file_path: str = None, env_name: str = None) -> dict: | ||
| def _get_env(conda_env_file_path: str = None, env_name: str = None, platform: ModelFramework = None) -> dict: |
There was a problem hiding this comment.
Can you explain the changes in this file?
There was a problem hiding this comment.
if we pack the conda environment, we need to add mlserver dependencies if not defined in conda.yaml
In the case of mlflow this is the case as the model conda.yaml generated will not have anything related to mlserver as not required for training.
There is logic to add mlserver package in _add_required_deps however we also need the corresponding runtime to be packaged (e.g. mlserver-mlflow in this case) and it is dependant on the type of model used. A reference to the platform is able to let us do that.
There is logic that
|
Should we add an associated example with MLFlow? |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This PR adds various fixes that we needed to get an mlflow model(e.g based on pytorch) saved and deployed using Tempo.
The main change is to pack the conda environment if
save_envisTrueeven if there is no custom predict function (i.e.BaseModel._user_funcisFalse) defined, which is the case with individual models.Other changes that are required as well:
Pack the runtime mlserver dependency as well, for example in the case of mlfow runtime we need to pack
mlserver-mlflow. Models haveplatformwhich we use to specify with platform we use. (ModelFramework.MLFlow->mlserver-mlflow) and that is defined inMLServerRuntimeEnvDeps.MLServer supports a numpy codec for request input when the whole input in the (case of an image) is being sent in one go as in the first input. Setting
content_type=npin the inference request will trigger that. check: Add request codec for numpy and strings which returns the first element MLServer#286 .There is an example to showcase the pytorch model serving as a notebook.
TODO in this PR still: