Forward rest parameters to model#1921
Conversation
idlefella
changed the title
| params = None | ||
| if payload.parameters and payload.parameters.model_extra: | ||
| params = payload.parameters.model_extra | ||
| model_output = self._model.predict(decoded_payload, params=params) |
There was a problem hiding this comment.
is there a test that can be added for this new functionality @idlefella ?
There was a problem hiding this comment.
@idlefella I think params is only supported for class-based pyfunc models and therefore this proposed changed will not work for function-based models.
I think we need to find a way to check the type of the model loaded and not pass params for function-based models.
check here
There was a problem hiding this comment.
Hi @sakoush
Thanks for the comment. Ok then I will try to write a test for class-based and function-based models and see how or if these parameters can be forwarded. I hope to find time in the next couple of days.
Thanks!
There was a problem hiding this comment.
Hi @sakoush, when the underlying model is a python function, then the function is wrapped into a _FunctionPythonModel:
https://github.com/mlflow/mlflow/blob/7d5dcf7e9b59c0ed8799024cae9c054634c616cd/mlflow/pyfunc/model.py#L142C1-L175C38
When params are set, the class prints a warning. So this doesn't cause any issues.
I also added some unittests for the forwarded params.
sakoush
left a comment
sakoush
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your contribution @idlefella!
2 participants
Pull Request
Fixes #1660
Description
The parameters sent over REST to the mlserver are not forwarded to the predict function of the model. This PR fixes that issue.
Changes Made
Properly forward the params to the predict function of the model.
Related Issues
Screenshots (if applicable)
Checklist
Additional Notes