aws / aws/sagemaker-python-sdk

Setting AsyncPredictor.deserializer doesn't work

Open
#3,100 1 comment 0 reactions 1 assignee Claimed by @nargokul View on GitHub
component: async inference type: bug
Dominant language
Python
Stars
2.3k
Forks
1.3k
Avg merge
1d 22h
Merged PRs (30d)
35

Description

**Describe the bug**

An `AsyncPredictor` (returned when deploying an async inference endpoint) exposes `.serializer` and `.deserializer` properties like a typical `Predictor` does... But behaviour of these properties is not consistent in async because of the way the class wraps around an internal `.predictor`.

Overriding a predictor's `.serializer` and `.deserializer` properties after creation is useful and expected functionality (IMO) because:

1. As far as I understand, the current de/serializer class architecture requires setting content_type/accept independently of individual requests. Therefore the only way to switch sent or received content type is by updating the de/serializer or re-creating the entire predictor.
2. In the past, not all predictor classes have supported `serializer` and `deserializer` constructor arguments (see #1997): So some samples have expected to set these properties after constructing.

**To reproduce**

```python
# Create some async predictor:
predictor = some_pytorch_model.deploy(
async_inference_config=sagemaker.async_inference.AsyncInferenceConfig(
output_path="s3://doc-example-bucket/folder",
max_concurrent_invocations_per_instance=2,
),
)

# Override de/serializers (PyTorch defaults to Numpy):
predictor.serializer = sagemaker.serializers.JsonSerializer()
predictor.deserializer = sagemaker.deserializers.JsonDeserializer()

# Make a prediction:
resp = predictor.predict({ "hi": "there" })
```

**Expected behavior**

The endpoint receives a request with `ContentType` and `Accept` matching the configured serializers (`application/json` in the above example).

**Actual behavior**

Because AsyncPredictor [uses its own serializer](https://github.com/aws/sagemaker-python-sdk/blob/5b8eb10bfddcb54fb65c8b28f923697619672120/src/sagemaker/predictor_async.py#L169), the input request is as expected.

...But because it [uses .predictor property's deserializer](https://github.com/aws/sagemaker-python-sdk/blob/5b8eb10bfddcb54fb65c8b28f923697619672120/src/sagemaker/predictor_async.py#L190), the overrides do not affect the response: default NumpyDeserializer and `application/x-npy` Accept headers are still used.

**Screenshots or logs**

N/A

**System information**
A description of your system. Please provide:
- **SageMaker Python SDK version**: 2.86.2 (but checked problem seems to still affect `master` - see links)
- **Framework name (eg. PyTorch) or algorithm (eg. KMeans)**: PyTorch (but should be general)
- **Framework version**: 1.10
- **Python version**: py3.8
- **CPU or GPU**: CPU
- **Custom Docker image (Y/N)**: N

**Additional context**

Today we can work around this by overriding de/serializers on both the outer (async) and inner (sync) predictor objects, as follows:

```python
async_predictor.serializer = JsonSerializer()
async_predictor.deserializer = JsonDeserializer()
async_predictor.predictor.serializer = async_predictor.serializer
async_predictor.predictor.deserializer = async_predictor.deserializer
```

...But I'd recommend a better solution would be to make `AsyncPredictor.serializer` and `AsyncPredictor.deserializer` into `@property`s that just read from and write to the inner `.predictor.(de)serializer`?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.