[Support inference params-4] Support inference params for model serving by serena-ruan · Pull Request #8976 · mlflow/mlflow

serena-ruan · 2023-07-06T06:54:25Z

Related Issues/PRs

Depends on #8974

What changes are proposed in this pull request?

Support passing params as part of payload for model serving

How is this patch tested?

Existing unit/integration tests
New unit/integration tests
Manual tests (describe details, including test results, below)

Does this PR change the documentation?

No. You can skip the rest of this section.
Yes. Make sure the changed pages / sections render correctly in the documentation preview.

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

Support passing params as part of payload for model serving

What component(s), interfaces, languages, and integrations does this PR affect?

Components

Interface

area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
area/windows: Windows support

Language

language/r: R APIs and clients
language/java: Java APIs and clients
language/new: Proposals for new client languages

Integrations

integrations/azure: Azure and Azure ML integrations
integrations/sagemaker: SageMaker integrations
integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Signed-off-by: Serena Ruan <[email protected]>

mlflow-automation · 2023-07-06T07:19:36Z

Documentation preview for 7f123d6 will be available here when this CircleCI job completes successfully.

More info

Ignore this comment if this PR does not change the documentation.
It takes a few minutes for the preview to be available.
The preview is updated when a new commit is pushed to this PR.
This comment was created by https://github.com/mlflow/mlflow/actions/runs/5584686460.

Signed-off-by: Serena Ruan <[email protected]>

BenWilson2

LGTM! Thanks @serena-ruan :)

WeichenXu123 · 2023-07-12T00:54:37Z

+                if isinstance(datetime_value, int):
+                    raise MlflowException.invalid_parameter_value(
+                        f"Invalid value for param {name}, it should "
+                        f"be convertible to datetime.date/datetime, got {value}"
+                    )


I think we don't need this because datetime_value = np.datetime64(value).item() already converts the value ?

The strange thing is that np.datetime64('20230701').item() is actually an integer 😅

WeichenXu123

LGTM but one minor comment

Signed-off-by: Serena Ruan <[email protected]>

harupy · 2023-07-12T02:02:36Z

        "datetime_param"
    ] == np.datetime64("2023-06-26 00:00:00")

+    # Test model serving


Not a blocker, but we should consider splitting this giant test. A long test is evil because:

Harder to tell what's being tested.

Harder to tell what's going on.

More code = More likely to make mistakes.

Harder to update.

Takes longer to run.

Signed-off-by: Serena Ruan <[email protected]>

harupy · 2023-07-12T08:19:25Z

+    prediction = json.loads(response.content.decode("utf-8"))["predictions"]
+    for param, value in test_params.items():
+        if param == "double_array":
+            assert (prediction[param] == value).all()


Not a blocker, but when cheking a numpy array or pandas dataframe, we should use np.testing.assert_array_equal or pandas.testting.assert_frame_equal. assert (prediction[param] == value).all() tells us True or False, but assert_xxx_yyy functions gives more information which is helpful for debugging.

harupy

LGTM!

Signed-off-by: Serena Ruan <[email protected]>

dbczumar

@serena-ruan @harupy @BenWilson2

Looks like we don't have backwards compatibility with old models. if we release this and users try deploying old models created before MLflow 2.5.0, they won't be able to score them (huge potential incident)

Here's how I tested:

Check out branch-2.4
Create a pyfunc model without params and log it:

import mlflow
from mlflow.models import infer_signature

import numpy as np

# s = infer_signature(["k"], params={"foo": np.datetime64("323432"), "bar": [True, False]})
s = infer_signature(["k"])


class MyMod(mlflow.pyfunc.PythonModel):
    def predict(self, ctx, inp):
        return [7 for item in inp]


with mlflow.start_run():
    m = mlflow.pyfunc.log_model(
        python_model=MyMod(),
        artifact_path="foo",
        signature=s,
        pip_requirements=[],
        registered_model_name="testmodel",
    )
    print(m.model_uri)
    print(m.signature)

m = mlflow.pyfunc.load_model(m.model_uri)
print("TYPE", type(m))
print(hasattr(m, 'predict'))

Check out the PR branch
Serve the model

mlflow models serve -m runs:/c134dd5a60084efa903797b66733cffd/foo --env-manager local

Query the model:

import requests

resp = requests.post(
    url="http://127.0.0.1:8000/invocations",
    json={
        "dataframe_records": [{"test": "input"}]
    },
)

print(resp)
print(resp.text)

The following error occurs:

{"error_code": "BAD_REQUEST", "message": "Encountered an unexpected error while evaluating the model. Verify that the serialized input Dataframe is compatible with the model for inference.", "stack_trace": "Traceback (most recent call last):\n  File \"/Users/corey.zumar/mlflow/mlflow/pyfunc/scoring_server/__init__.py\", line 276, in transformation\n    charset = parameter_values.get(\"charset\", \"utf-8\").lower()\n  File \"/Users/corey.zumar/mlflow/mlflow/pyfunc/__init__.py\", line 428, in predict\n    column values will be cast as the required tensor spec type.\n  File \"/Users/corey.zumar/mlflow/mlflow/pyfunc/model.py\", line 365, in predict\nTypeError: predict() missing 1 required positional argument: 'params'\n"}

If we've already checked in code with a similar problem on Databricks, can we revert it for now? I want to make sure we aren't introducing serious regressions here

Signed-off-by: Serena Ruan <[email protected]>

serena-ruan · 2023-07-13T04:04:11Z

If we've already checked in code with a similar problem on Databricks, can we revert it for now? I want to make sure we aren't introducing serious regressions here

This is so strange.. I added unit test for python model 38e1c15#diff-592813f9fb198dda0982794f2221a408f316709190b3bf02c0e77eb2c1107075, I also manually tested your approach locally, but mine works fine 🤔

mlflow models serve -m runs:/3f87fdff03524c19908c3a47fb99f9cd/test_model --env-manager local

And my MLmodel file looks like this:

artifact_path: test_model
flavors:
  python_function:
    cloudpickle_version: 2.2.1
    env:
      conda: conda.yaml
      virtualenv: python_env.yaml
    loader_module: mlflow.pyfunc.model
    python_model: python_model.pkl
    python_version: 3.8.16
mlflow_version: 2.4.0
model_uuid: 3cbde93be0114644a6ec900c64cab39d
run_id: 3f87fdff03524c19908c3a47fb99f9cd
signature:
  inputs: '[{"type": "string"}]'
  outputs: null
utc_time_created: '2023-07-13 01:29:55.467561'

So I'm wondering how could I reproduce your error 🤔

Signed-off-by: Serena Ruan <[email protected]>

BenWilson2 · 2023-07-17T15:58:52Z

Can we do a manual validation of spark_udf (mlflow 2.3.x model, use as spark_udf after installing this branch)? I'm really interested on seeing how the udf logic will handle the optional params component with this version difference.

harupy · 2023-07-18T07:12:56Z

@dbczumar @serena-ruan I tried what's described in #8976 (review) and it worked.

serena-ruan · 2023-07-18T07:14:07Z

@dbczumar @serena-ruan I tried what's described in #8976 (review) and it worked.

Thanks Haru!

dbczumar

@serena-ruan I've confirmed that we have backwards compatibility for model serving, batch inference, and spark UDF.

I found one small issue where the following code throws an error, but I'd expect it to work (let me know if I'm missing something):

s = infer_signature(["k"], params={"foo": np.datetime64("323432"), "bar": [True, False]})

mlflow.exceptions.MlflowException: Failed to infer schema for parameters: [('foo', numpy.datetime64('323432'), MlflowException("Invalid value for param 'foo': datetime (default: 323432) with shape None, it should be convertible to datetime.date/datetime, got 323432"))]

Otherwise, LGTM and feel free to merge! Thanks so much, @serena-ruan !

harupy · 2023-07-18T07:27:40Z

-    # spark_master = os.environ.get("SPARK_MASTER", "local[1]")
+    spark_master = os.environ.get("SPARK_MASTER", "local[1]")
    # If doing local testing, comment-out the following line.
-    spark_master = os.environ.get("SPARK_MASTER", "local-cluster[2, 1, 1024]")
+    # spark_master = os.environ.get("SPARK_MASTER", "local-cluster[2, 1, 1024]")


Do we need this change?

Ah I forgot to change it back for local testing! I'll change it back

serena-ruan · 2023-07-18T07:28:59Z

@serena-ruan I've confirmed that we have backwards compatibility for model serving, batch inference, and spark UDF.

I found one small issue where the following code throws an error, but I'd expect it to work (let me know if I'm missing something):
s = infer_signature(["k"], params={"foo": np.datetime64("323432"), "bar": [True, False]})
mlflow.exceptions.MlflowException: Failed to infer schema for parameters: [('foo', numpy.datetime64('323432'), MlflowException("Invalid value for param 'foo': datetime (default: 323432) with shape None, it should be convertible to datetime.date/datetime, got 323432"))]
Otherwise, LGTM and feel free to merge! Thanks so much, @serena-ruan !

This is expected, because when we convert numpy.datetime64('323432') to datetime.datetime, it actually becomes a number, which means it's not a valid timestamp, so we throw the error, and this is added to test case as well :)

Signed-off-by: Serena Ruan <[email protected]>

Co-authored-by: Harutaka Kawamura <[email protected]> Signed-off-by: Serena Ruan <[email protected]>

Signed-off-by: Serena Ruan <[email protected]>

support params for scoring_server

1e51785

Signed-off-by: Serena Ruan <[email protected]>

serena-ruan mentioned this pull request Jul 6, 2023

[Support inference params-5]Add docs for inference params #8977

Merged

33 tasks

github-actions Bot added area/scoring MLflow Model server, model deployment tools, Spark UDFs rn/feature Mention under Features in Changelogs. labels Jul 6, 2023

harupy added the only-latest If applied, only test the latest version of each group in cross-version tests. label Jul 6, 2023

serena-ruan marked this pull request as draft July 7, 2023 00:16

merge master

e214eb9

Signed-off-by: Serena Ruan <[email protected]>