Sherif akoush/mlsv 42/add alibi explain runtime #320

sakoush · 2021-09-23T16:17:32Z

This PR adds the initial support to run alibi explain models using mlserver. We treat alibi explain models similar to inference models with respect to deployment.

We support blackbox and whitebox explainers: https://github.com/SeldonIO/alibi/blob/master/doc/source/overview/white_box_black_box.ipynb

In the case of blackbox explainers, there should be an inference model that is deployed and exposed via an endpoint that implements the v2 protocol: https://github.com/kserve/kserve/tree/master/docs/predict-api/v2 for now we only support REST.

In the case of whitebox explainers, the explainer needs to load the "full" model in the same process space as it needs access to the model internals (such as gradients in the case of IntegratedGradients)

We have the ability to create an explainer on the fly with the correct config settings in addition to loading persisted explainer artifacts.

There are some edge cases that are yet to be sorted out so this is still considered experimental at this stage.

adriangonz · 2021-09-24T14:31:08Z

mlserver/codecs/numpy.py

@@ -93,11 +93,11 @@ def encode(cls, name: str, payload: np.ndarray) -> ResponseOutput:
        )

    @classmethod
-    def decode(cls, request_input: RequestInput) -> np.ndarray:
-        model_data = _to_ndarray(request_input)
+    def decode(cls, v2_data: Union[RequestInput, ResponseOutput]) -> np.ndarray:


Interesting! Do you think we should make this the new interface for the InputCodec class? I was thinking that we could also include the opposite conversion, although that would probably require splitting the existing encode method into encode_response and encode_request.

runtimes/alibi-explain/mlserver_alibi_explain/common.py

adriangonz · 2021-09-24T14:45:16Z

runtimes/alibi-explain/mlserver_alibi_explain/runtime.py

+        return np_codec.decode(v2_response.outputs[0])  # type: ignore # TODO: fix mypy and first output
+
+    @abc.abstractmethod
+    def _explain_impl(self, input_data: Any, settings: BaseSettings) -> Explanation:


Would there be any scenario where we wouldn't want to execute it asynchronously?

no but I thought that implementation should not care about that and we get the logic and execute it async in base class

adriangonz · 2021-09-24T14:46:00Z

runtimes/alibi-explain/mlserver_alibi_explain/common.py

+
+
+# TODO: is this inference response?
+def create_v2_from_any(data: Any, name: str) -> Dict:


Should we wrap this into a Codec? Conscious that it's something kind of temporary, but could be maybe useful in other cases?

I agree, these are the type of components that we'll end up also needing in Tempo if we want to easily create clients that can convert to/from explainers

there is StringCodec.encode that achieves the same behaviour, so I am using it instead now.
Decoding the outout (e,g in the case of tempo and in the case of alibi runtime for the underlying remote inference request is going to be a separate ticket as discussed).

+ add factory capability in alibi

…sses

adriangonz

Nice one @sakoush! I've added a few comments here and there, although they are mainly around code conventions. Functionally, all looks good 👍

I haven't managed to look at the tests yet, but will have a look as soon as I can.

adriangonz · 2021-10-14T14:31:34Z

mlserver/codecs/string.py

+            name=data.name,
+            datatype=data.datatype,
+            shape=data.shape,
+            data=data.data.__root__.decode("ascii"),  # to allow json serialisation


Wouldn't this break serialisation into protobufs? Although we should probably leave that point outside of this PR TBH, as it's not clear how to make something compatible for both.

Besides that, should we decode it as utf8 instead of ascii?

this is not removed and we will just pass the List[str]
As discussed this will only work with json / REST

adriangonz · 2021-10-14T14:32:22Z

runtimes/alibi-detect/mlserver_alibi_detect/runtime.py

@@ -26,7 +26,7 @@ class Config:
        env_prefix = ENV_PREFIX_ALIBI_DETECT_SETTINGS

    init_detector: bool = False
-    detector_type: PyObject = ""
+    detector_type: PyObject = ""  # type: ignore


If we set it to None, would we also need the type: ignore comment?

No, because we need also to set it as Optional. this has been used elsewhere as well.

adriangonz · 2021-10-14T14:33:15Z

runtimes/alibi-detect/mlserver_alibi_detect/runtime.py

@@ -47,7 +47,8 @@ class AlibiDetectRuntime(MLModel):

    def __init__(self, settings: ModelSettings):

-        self.alibi_detect_settings = AlibiDetectSettings(**settings.parameters.extra)
+        extra = settings.parameters.extra  # type: ignore


Why is that type: ignore required? Is it because parameters may be None? If that's the case, we could check here whether it's set, and if not just default to an empty AlibiDetectSettings() object (which could still fetch values from the environment).

good point.

adriangonz · 2021-10-14T14:36:32Z

runtimes/alibi-explain/mlserver_alibi_explain/common.py

+    integrated_gradients = _INTEGRATED_GRADIENTS_TAG
+
+
+def convert_from_bytes(output: ResponseOutput, ty: Optional[Type]) -> Any:


Is the convert_from_bytes method still needed here now that we've got the extra methods on the StringCodec?

It is used in tests to recover the data from explain calls.

This utility is not yet moved to StringCodec. I though I will leave it as TODO given that it is not strictly required.

Got it! Could you add a #TODO comment so that it's clear why we decided to leave it here?

runtimes/alibi-explain/mlserver_alibi_explain/common.py

adriangonz · 2021-10-14T14:54:32Z

runtimes/alibi-explain/mlserver_alibi_explain/explainers/white_box_runtime.py

+            model_parameters: Optional[ModelParameters] = self.settings.parameters
+            assert model_parameters is not None
+            uri = model_parameters.uri  # type ignore
+            assert uri is not None, "uri has to be set"


Do we have a uri in this case?

adriangonz · 2021-10-14T14:56:45Z

runtimes/alibi-explain/mlserver_alibi_explain/runtime.py

+            explain_parameters=explain_parameters,
+        )
+        # TODO: Convert alibi-explain output to v2 protocol, for now we use to_json
+        return StringCodec.encode(payload=[explanation.to_json()], name="explain")


nit: should we call the output head explanation instead of explain?

adriangonz · 2021-10-14T14:58:05Z

runtimes/alibi-explain/mlserver_alibi_explain/runtime.py

+        # TODO: we probably want to validate the enum more sanely here
+        # we do not want to construct a specific alibi settings here because
+        # it might be dependent on type
+        # although at the moment we only have one `AlibiExplainSettings`


We could have a separate class to manage the current enum and tuple dict. This could help with abstracting some of those details there.

moved them to a different module for now.

runtimes/alibi-explain/mlserver_alibi_explain/runtime.py

adriangonz · 2021-10-14T15:01:13Z

runtimes/alibi-explain/requirements-dev.txt

@@ -0,0 +1,4 @@
+tensorflow
+numpy
+nest_asyncio


Nice! Are you using nest_asyncio? Although, is it only for tests?

only in tests for now.

runtimes/alibi-explain/mlserver_alibi_explain/alibi_dependency_reference.py

adriangonz · 2021-10-15T15:35:45Z

runtimes/alibi-explain/mlserver_alibi_explain/common.py

+    integrated_gradients = _INTEGRATED_GRADIENTS_TAG
+
+
+def convert_from_bytes(output: ResponseOutput, ty: Optional[Type]) -> Any:


Got it! Could you add a #TODO comment so that it's clear why we decided to leave it here?

runtimes/alibi-explain/mlserver_alibi_explain/common.py

adriangonz · 2021-10-15T15:41:51Z

runtimes/alibi-explain/tests/conftest.py

+TESTS_PATH = Path(os.path.dirname(__file__))
+
+
+# TODO: how to make this in utils?


Yeah, it's annoying to remember adding this every time. Not sure whether it would be possible to have this on a pytest plugin or something similar?

adriangonz · 2021-10-15T15:42:46Z

runtimes/alibi-explain/tests/conftest.py

+        "parallel_workers": 0,
+    }
+
+    model_settings_path.write_text(json.dumps(model_settings_dict, indent=4))


Nice one! I didn't know write_text existed.

runtimes/alibi-explain/tests/conftest.py

runtimes/alibi-explain/tests/test_alibi_runtime_base.py

adriangonz · 2021-10-15T15:48:28Z

tests/codecs/test_numpy.py

+    request_input_result = codec.encode_request_input(name="foo", payload=decoded)
+    assert response_output.datatype == request_input_result.datatype
+    assert response_output.shape == request_input_result.shape
+    assert response_output.data == request_input_result.data
+    assert request_input_result.parameters.content_type == codec.ContentType


Should we move these asserts to a separate test case to ensure each test only tests one thing?

perhaps although it feels fine in this case as we are checking the content of the encoded object.

this pattern is used elsewhere as well.

I'm usually more keen on having tests that only test a single thing. This is generally a good pattern within pytest (and general unit testing), as it also makes tests smaller and easier to maintain.

Don't want to hold off the PR just for this nitpick though, but I think we should split them at a some point down the line.

sakoush marked this pull request as draft September 23, 2021 16:17

adriangonz reviewed Sep 24, 2021

View reviewed changes

ukclivecox mentioned this pull request Sep 29, 2021

Custom explainability models SeldonIO/seldon-core#1832

Closed

sakoush force-pushed the SherifAkoush/MLSV-42/add_alibi_explain_runtime branch 2 times, most recently from 8668c45 to ae6aff1 Compare October 11, 2021 09:29

sakoush added 25 commits October 12, 2021 11:22

initial project skeleton for alibi-explain runtime

1e592ce

POC commit

9c10981

POC prepare testing

43b1db6

disable testing

e472624

WIP

af1810e

wiring up predict fn

36acbc5

anchor image with local model

56d4552

add a hacky test for remote_predict

d439648

allow numpy codec for v2 output

7bd1bb9

MNIST working?

85eb8fb

tidy up

b469a3f

wire up explain parameters (POC)

d90e061

add the ability to load an anchor from disk

2f48edb

push test artifacts

a9ffe5b

remove one TODO!

1657a2a

pass kwargs instead

d82b0ee

defaults are set in alibi

f519352

POC integrated gradients

44698e9

POC integrated gradients with infernece model as uri

399bb4a

abstract explainers into bb and wb

0f420aa

+ add factory capability in alibi

minor refactor to common utils

5ff6390

add integrated gradients via factory

bfc4d2a

add a test to make sure we can import the strings that define the cla…

a3eb52d

…sses

add support for anchor text

6cf0fe1

tweaks to get things working

7b535db

sakoush requested review from axsaucedo and adriangonz October 12, 2021 10:52

sakoush added 11 commits October 13, 2021 18:30

fixes for anchor text working

1af15b6

add a test for numpy codec

1cc91c9

tidy up numpy codec

e82e1cf

add tests for str codec

588f178

refactor and add a test for black box v2 inference request encoding

e396277

add more tests

fc1c255

use assert_array_almost_equal

402df44

refactor to fix tests

acfb093

fmt

012f620

add a test for explain parameters pass through

5d0b7a4

check also data is wired through properly

7eba4a0

adriangonz reviewed Oct 14, 2021

View reviewed changes

sakoush added 11 commits October 15, 2021 09:24

add a test for explain parameters pass through

ed5f2a3

set default alibi detect settings

602cd1f

use mlserver errors

6925b0a

make alibi dependency reference mode readable

81a296c

add exceptions for black box explainers

3720a26

and for whitebox

e525b56

share common code for loading explainers from disk

b992a30

revert param rename

66c0080

check inference uri in init

5b433d9

tidy up exception handling

9afd2aa

PR comments

6d272be

adriangonz reviewed Oct 15, 2021

View reviewed changes

sakoush added 2 commits October 15, 2021 17:04

add anchor tabular

03ad825

PR comments

8f449ff

adriangonz approved these changes Oct 15, 2021

View reviewed changes

sakoush merged commit 565b3b2 into SeldonIO:master Oct 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sherif akoush/mlsv 42/add alibi explain runtime #320

Sherif akoush/mlsv 42/add alibi explain runtime #320

sakoush commented Sep 23, 2021 •

edited

Loading

adriangonz Sep 24, 2021

adriangonz Sep 24, 2021

sakoush Sep 24, 2021

adriangonz Sep 24, 2021

axsaucedo Sep 30, 2021

sakoush Oct 11, 2021

adriangonz left a comment

adriangonz Oct 14, 2021

sakoush Oct 15, 2021

adriangonz Oct 14, 2021

sakoush Oct 15, 2021

adriangonz Oct 14, 2021

sakoush Oct 15, 2021

adriangonz Oct 14, 2021

sakoush Oct 15, 2021

adriangonz Oct 15, 2021

adriangonz Oct 14, 2021

adriangonz Oct 14, 2021

adriangonz Oct 14, 2021

sakoush Oct 15, 2021

adriangonz Oct 14, 2021

sakoush Oct 15, 2021

adriangonz Oct 15, 2021

adriangonz Oct 15, 2021

adriangonz Oct 15, 2021

adriangonz Oct 15, 2021

sakoush Oct 15, 2021

adriangonz Oct 15, 2021



		# TODO: is this inference response?
		def create_v2_from_any(data: Any, name: str) -> Dict:

		integrated_gradients = _INTEGRATED_GRADIENTS_TAG


		def convert_from_bytes(output: ResponseOutput, ty: Optional[Type]) -> Any:

		TESTS_PATH = Path(os.path.dirname(__file__))


		# TODO: how to make this in utils?

Sherif akoush/mlsv 42/add alibi explain runtime #320

Sherif akoush/mlsv 42/add alibi explain runtime #320

Conversation

sakoush commented Sep 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adriangonz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakoush commented Sep 23, 2021 •

edited

Loading