[Support inference params-5]Add docs for inference params by serena-ruan · Pull Request #8977 · mlflow/mlflow

serena-ruan · 2023-07-06T06:55:59Z

Related Issues/PRs

Depends on #8976

What changes are proposed in this pull request?

Add docs for inference params and new ModelSignature

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.

Add docs for supporting params in model inference.

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

mlflow-automation · 2023-07-06T07:58:04Z

Documentation preview for eebc0da 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/5617039612.

BenWilson2 · 2023-07-11T17:21:24Z

+a shape of ``(-1,)``. If the parameter's type or shape is incompatible, an exception will be raised. 
+Additionally, the value of the parameter is validated against the specified type in the signature. We attempt 
+to convert the value to the specified type, and if this conversion fails, an MlflowException will be raised.
+


Can we provide a list with generic examples of what parameter types are supported?
Can there also be an explanation added of why this feature exists and how it can be used as an intro to a new section above this? This is a pretty important and long asked-for feature that I think another ^^^^^ section is warranted to explain that inference parameters are supported now :)

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

harupy · 2023-07-19T09:06:51Z

@serena-ruan Can you push a commit? I want to see the doc preview.

serena-ruan · 2023-07-19T09:09:09Z

@serena-ruan Can you push a commit? I want to see the doc preview.

Can I just rerun https://github.com/mlflow/mlflow/actions/runs/5596007324 this job?

harupy · 2023-07-19T09:30:28Z

Can I just rerun https://github.com/mlflow/mlflow/actions/runs/5596007324 this job?

Yes, but I'm not sure if it's rerunable.

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

harupy · 2023-07-19T13:46:45Z

+    from mlflow.models import infer_signature
+    from mlflow.transformers import generate_signature_output
+
+    # Infer the signature
+    signature = infer_signature(
+        data,
+        generate_signature_output(model, data),
+    )
+
+    # Define an inference_config
+    inference_config = {
+        "num_beams": 5,
+        "max_length": 30,
+        "do_sample": True,
+        "remove_invalid_values": True,
+    }
+
+    # Saving inference_config with the model
+    mlflow.transformers.save_model(
+        model,
+        path=model_path,
+        inference_config=inference_config,
+        signature=signature,
+    )
+
+    pyfunc_loaded = mlflow.pyfunc.load_model(model_path)
+    # inference_config will be applied
+    result = pyfunc_loaded.predict(data)


This example contains undefined variables and modules. Can we add them?

I think a long complete example is better than a short incomplete example.

harupy · 2023-07-19T13:58:40Z

+    from mlflow.models import infer_signature
+    import transformers
+
+    architecture = "mrm8488/t5-base-finetuned-common_gen"


How large is this model?

roughly 1GB

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

BenWilson2

Some very minor nits to address, otherwise looks great! Thanks @serena-ruan :)

harupy

LGTM!

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

serena-ruan · 2023-07-20T08:27:37Z

-            params_schema = model_metadata.get_params_schema()
-            return _enforce_params_schema(params, params_schema)
-        else:
+    if hasattr(model_metadata, "get_params_schema"):


@harupy I fixed this to make sure if user doesn't pass params the default values are still applied, and added two tests in test_pyfunc_schema_enforcement.py, could you pls review again? Thanks 😄

harupy · 2023-07-20T09:19:05Z

+    else:
+        if params:


Suggested change

else:

if params:

if params:

I think we can remove this else.

harupy · 2023-07-20T09:20:12Z

+def test_enforce_params_schema_errors_with_python_model():
+    class MyModel(mlflow.pyfunc.PythonModel):
+        def predict(self, ctx, model_input, params=None):
+            return list(params.values()) if isinstance(params, dict) else None
+
+    params = {"str_param": "string", "int_array": [1, 2, 3], "123": 123}
+    signature = infer_signature(["input"])
+    signature_with_params = infer_signature(["input"], params=params)
+
+    with mlflow.start_run():
+        model_info = mlflow.pyfunc.log_model(
+            python_model=MyModel(), artifact_path="model1", signature=signature
+        )
+        model_info_with_params = mlflow.pyfunc.log_model(
+            python_model=MyModel(), artifact_path="model2", signature=signature_with_params
+        )
+
+    loaded_model = mlflow.pyfunc.load_model(model_info.model_uri)
+
+    with pytest.raises(
+        MlflowException,
+        match=r"`params` can only be specified at inference time if the model signature",
+    ):
+        loaded_model.predict(["input"], params=params)
+
+    loaded_model_with_params = mlflow.pyfunc.load_model(model_info_with_params.model_uri)
+    with pytest.raises(MlflowException, match=r"Parameters must be a dictionary. Got type 'list'"):
+        loaded_model_with_params.predict(["input"], params=[1, 2, 3])
+
+    with mock.patch("mlflow.models.utils._logger.warning") as mock_warning:
+        loaded_model_with_params.predict(["input"], params={123: 456})
+    mock_warning.assert_called_with(
+        "Keys in parameters should be of type `str`, but received non-string keys."
+        "Converting all keys to string..."
+    )


/ai split this test into two

@harupy Sure, here are the two tests split:

Test 1

def test_enforce_params_schema_errors_with_model_without_params(): class MyModel(mlflow.pyfunc.PythonModel): def predict(self, ctx, model_input, params=None): return list(params.values()) if isinstance(params, dict) else None params = {"str_param": "string", "int_array": [1, 2, 3], "123": 123} signature = infer_signature(["input"]) with mlflow.start_run(): model_info = mlflow.pyfunc.log_model( python_model=MyModel(), artifact_path="model1", signature=signature ) loaded_model = mlflow.pyfunc.load_model(model_info.model_uri) with pytest.raises( MlflowException, match=r"`params` can only be specified at inference time if the model signature", ): loaded_model.predict(["input"], params=params)

Test 2

def test_enforce_params_schema_errors_with_model_with_params(): class MyModel(mlflow.pyfunc.PythonModel): def predict(self, ctx, model_input, params=None): return list(params.values()) if isinstance(params, dict) else None params = {"str_param": "string", "int_array": [1, 2, 3], "123": 123} signature_with_params = infer_signature(["input"], params=params) with mlflow.start_run(): model_info_with_params = mlflow.pyfunc.log_model( python_model=MyModel(), artifact_path="model2", signature=signature_with_params ) loaded_model_with_params = mlflow.pyfunc.load_model(model_info_with_params.model_uri) with pytest.raises(MlflowException, match=r"Parameters must be a dictionary. Got type 'list'"): loaded_model_with_params.predict(["input"], params=[1, 2, 3]) with mock.patch("mlflow.models.utils._logger.warning") as mock_warning: loaded_model_with_params.predict(["input"], params={123: 456}) mock_warning.assert_called_with( "Keys in parameters should be of type `str`, but received non-string keys." "Converting all keys to string..." )

Click here to see the prompt and usage data

Prompt:

split this test into two ``` def test_enforce_params_schema_errors_with_python_model(): class MyModel(mlflow.pyfunc.PythonModel): def predict(self, ctx, model_input, params=None): return list(params.values()) if isinstance(params, dict) else None params = {"str_param": "string", "int_array": [1, 2, 3], "123": 123} signature = infer_signature(["input"]) signature_with_params = infer_signature(["input"], params=params) with mlflow.start_run(): model_info = mlflow.pyfunc.log_model( python_model=MyModel(), artifact_path="model1", signature=signature ) model_info_with_params = mlflow.pyfunc.log_model( python_model=MyModel(), artifact_path="model2", signature=signature_with_params ) loaded_model = mlflow.pyfunc.load_model(model_info.model_uri) with pytest.raises( MlflowException, match=r"`params` can only be specified at inference time if the model signature", ): loaded_model.predict(["input"], params=params) loaded_model_with_params = mlflow.pyfunc.load_model(model_info_with_params.model_uri) with pytest.raises(MlflowException, match=r"Parameters must be a dictionary. Got type 'list'"): loaded_model_with_params.predict(["input"], params=[1, 2, 3]) with mock.patch("mlflow.models.utils._logger.warning") as mock_warning: loaded_model_with_params.predict(["input"], params={123: 456}) mock_warning.assert_called_with( "Keys in parameters should be of type `str`, but received non-string keys." "Converting all keys to string..." ) ```

Usage:

{ "prompt_tokens": 420, "completion_tokens": 474, "total_tokens": 894, "estimated_cost_in_usd": 0.04104 }

@serena-ruan Can we split this test?

harupy

re-LGTM!

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

github-actions Bot added area/docs Documentation issues rn/documentation Mention under Documentation Changes in Changelogs. labels Jul 6, 2023

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

BenWilson2 reviewed Jul 11, 2023

View reviewed changes

add docs

4c95aac

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

serena-ruan force-pushed the doc branch from 20ad11e to 4c95aac Compare July 19, 2023 06:58

serena-ruan changed the base branch from master to inf_params July 19, 2023 06:58

serena-ruan marked this pull request as ready for review July 19, 2023 06:59

serena-ruan requested review from BenWilson2 and harupy July 19, 2023 06:59

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

address comments

9bb082f

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

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

harupy reviewed Jul 19, 2023

View reviewed changes

address comments

c12da03

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

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

BenWilson2 reviewed Jul 20, 2023

View reviewed changes

Comment thread docs/source/models.rst Outdated

BenWilson2 approved these changes Jul 20, 2023

View reviewed changes

harupy approved these changes Jul 20, 2023

View reviewed changes

serena-ruan added 3 commits July 20, 2023 16:14

fix a small bug and add test cases for adding default values

351b6ef

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

address comments

ee0086e

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

add one more test for _enforce_params_schema

335f172

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

serena-ruan commented Jul 20, 2023

View reviewed changes

harupy reviewed Jul 20, 2023

View reviewed changes

harupy approved these changes Jul 20, 2023

View reviewed changes

serena-ruan added 2 commits July 20, 2023 17:36

remove else

006c1be

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

split test and fix an unstable result

eebc0da

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

serena-ruan merged commit c05cb51 into mlflow:inf_params Jul 21, 2023

serena-ruan deleted the doc branch July 21, 2023 02:19

Uh oh!

Conversation

serena-ruan commented Jul 6, 2023

Related Issues/PRs

What changes are proposed in this pull request?

How is this patch tested?

Does this PR change the documentation?

Release Notes

Is this a user-facing change?

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

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

Uh oh!

mlflow-automation commented Jul 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenWilson2 Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harupy commented Jul 19, 2023

Uh oh!

serena-ruan commented Jul 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harupy commented Jul 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BenWilson2 left a comment

Choose a reason for hiding this comment

Uh oh!

harupy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlflow Bot Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

harupy Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harupy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

mlflow-automation commented Jul 6, 2023 •

edited

Loading

BenWilson2 Jul 11, 2023 •

edited

Loading

harupy Jul 20, 2023 •

edited

Loading