Skip to content

fix mlserver infer with BYTES#1213

Merged
RafalSkolasinski merged 3 commits intoSeldonIO:masterfrom
RafalSkolasinski:batch-bytes
Jun 16, 2023
Merged

fix mlserver infer with BYTES#1213
RafalSkolasinski merged 3 commits intoSeldonIO:masterfrom
RafalSkolasinski:batch-bytes

Conversation

@RafalSkolasinski
Copy link
Copy Markdown
Contributor

Closes #1210

)
request_input_np = NumpyCodec.decode_input(request_input)

# Change datatype if BYTES to satisfy Tritonclient checks
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling this should be something handled in the NumpyCodec but it may be too tritonclient specific...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - do you know what's returned by NumpyCodec.decode_input at the moment?

Copy link
Copy Markdown
Contributor Author

@RafalSkolasinski RafalSkolasinski Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tritonclient expects

    elif np_dtype == np.object_ or np_dtype.type == np.bytes_:
        return "BYTES"
    return None

as otherwise it is None that was causing an issue.

As we do not set anything there explicitly (and I also seen that by adding extra logs earlier during debugging) we just have '<U1'

In [2]: import numpy as np

In [3]: x = np.array(["x"])

In [4]: x.dtype == np.bytes_
Out[4]: False

In [5]: x.dtype
Out[5]: dtype('<U1')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what's going is that the NumpyCodec encodes that list as a tensor of strings (i.e. using np.dtype(str)). However, Triton doesn't know how to handle that case.

if is_list_of(data, str):
# Handle special case of strings being treated as Numpy arrays
return np.dtype(str)

I can't remember why we had to add that np.dtype(str) special case, but I would be keen to keep it as is to avoid any potential side effects. Having said that, it's unclear whether changing the dtype here may cause any issues downstream (although I'd expect that strings in np arrays are quite an edge case).

Copy link
Copy Markdown
Contributor

@adriangonz adriangonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one @RafalSkolasinski! Added a small comment around tests - I think we can just reuse one of the existing fixtures, but everything else looks great. 👍

)
request_input_np = NumpyCodec.decode_input(request_input)

# Change datatype if BYTES to satisfy Tritonclient checks
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - do you know what's returned by NumpyCodec.decode_input at the moment?

Copy link
Copy Markdown
Contributor

@adriangonz adriangonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tweaking the fixture @RafalSkolasinski ! This should now be ready to go once the linter stops complaining (seems to be a black formatting issue - you can fix it with make fmt).

@RafalSkolasinski
Copy link
Copy Markdown
Contributor Author

@adriangonz done - should be fine now

@RafalSkolasinski RafalSkolasinski merged commit 07357ac into SeldonIO:master Jun 16, 2023
adriangonz pushed a commit that referenced this pull request Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

got unexpected datatype None from numpy array, expected BYTES" from mlserver infer

2 participants