Multi model deployment #208
Conversation
mii/client.py
Outdated
| deployments = [] | ||
| configs = mii.utils.import_score_file(deployment_tag).configs | ||
| for deployment in configs: | ||
| if not isinstance(configs[deployment], dict): |
There was a problem hiding this comment.
When the data is written to the score file it stores the dictionaries of all the deployments, along with the load balancer, model_path, and deployment_tag. The 'deployment' on line 18 in the for loop looks at all of them. So I determine if it is a model by checking if it is a dictionary
mii/deployment.py
Outdated
| if not deployments and not all((model, task, deployment_name)): | ||
| assert deployment_tag is not None, "Deployment tag must be set when starting empty deployment" | ||
| create_score_file(deployment_tag=deployment_tag, | ||
| deployment_type=deployment_type, | ||
| deployments=None, | ||
| model_path=model_path, | ||
| port_map=None, | ||
| lb_config=None) | ||
| return None |
There was a problem hiding this comment.
I changed it and made a new function, let me know if you think it is better
mii/config.py
Outdated
| mii_config: MIIConfig = MIIConfig.parse_obj({}) | ||
| ds_config: dict = None | ||
| version: int = 1 | ||
| deployed: bool = False |
There was a problem hiding this comment.
Removed the deployed option or make it hidden _deployed(verify this is hidden)
examples/multi_model/deploy.py
Outdated
| model=name, | ||
| deployment_name=name + "_deployment", | ||
| GPU_index_map=gpu_index_map3, | ||
| mii_configs=mii.config.MIIConfig(**mii_configs1))) |
There was a problem hiding this comment.
Are we not able to pass just the dictionary here?
| deployments, lb_config, model_path, port_map = _get_deployment_configs(deployment_tag) | ||
| mii_configs = None | ||
| if len(deployments) > 0: | ||
| mii_configs = getattr(next(iter(deployments.values())), |
There was a problem hiding this comment.
We really need to address this problem. MIIConfig class is required for each DeploymentConfig but the MIIConfig contains options that affect all models (e.g., restful API port). I believe we need to refactor the configs a bit to resolve this and have model-specific configs separate from deployment-specific configs.
mii/client.py
Outdated
| assert len(self.deployments) == 1, "Must pass deployment_name to query when using multiple deployments" | ||
| deployment = next(iter(self.deployments.values())) |
There was a problem hiding this comment.
If self.deployments has len of 1, then why do we need to do next(iter()) on it?
mii/config.py
Outdated
|
|
||
|
|
||
| class DeploymentConfig(BaseModel): | ||
| deployment_name: str = Field(alias="DEPLOYMENT_NAME_KEY") |
There was a problem hiding this comment.
I'm a little bit confused about these aliases. I know we discussed a change here last week, but I think there may have been some miscommunication.
mii/client.py
Outdated
| task=None, | ||
| model=None, | ||
| deployment_name=None, | ||
| enable_deepspeed=True, | ||
| enable_zero=False, | ||
| ds_config=None, | ||
| mii_config={}, | ||
| deployments=[], | ||
| deployment_type=DeploymentType.LOCAL, | ||
| model_path=None, | ||
| version=1): |
There was a problem hiding this comment.
Since this is a new interface, we do not need to support both the old and new way of adding models. Let's just allow passing of deployments and not task, model, deployment_name
mii/client.py
Outdated
| async def add_models_async(self, proto_request): | ||
| await getattr(self.lb_stub, "AddDeployment")(proto_request) | ||
|
|
||
| def add_models(self, |
There was a problem hiding this comment.
I'm a bit confused about the implementation of this method. It looks like we are re-generating the score file when we add models. And then we call init again? Does this not cause problems for models that are already running?
Co-authored-by: Michael Wyatt <[email protected]>
Co-authored-by: Michael Wyatt <[email protected]>
No description provided.