dianna icon indicating copy to clipboard operation
dianna copied to clipboard

Fix KernelSHAP that does not work with a model runner

Open gcroci2 opened this issue 2 years ago • 3 comments

Even if the function explain_image (see dianna/init.py) says that it accepts either a model or a function that runs the model as input, for KernelSHAP method this does not apply. Indeed, the explain_image method of KernelSHAP class seems to accept only the path to a ONNX model on disk (see here).

gcroci2 avatar Mar 29 '22 16:03 gcroci2

@geek-yang do you remember whether there was a specific reason for not allowing functions in KernelSHAP? Is it not possible or do we just have to add the runner = ... wrapper thing that the other models have as well?

If indeed it is not possible, we should raise an exception when people do try to use KernelSHAP with a function instead of a path to an onnx model.

egpbos avatar Mar 31 '22 12:03 egpbos

Hi @egpbos and @gcroci2 Sorry that I just noticed this thread. A short answer is, that we have already passed a runner function to shap and allowing functions from the high level API could be very difficult to fit into the current workflow due to the kernelshap implementation we use.

Here are the details:

For KernelSHAP we use the 'kernel explainer' from shap library as the basis. When working with images, it should receive a function including model, image segmentation and mask (see their example). It is not easy for the user to define everything themselves, therefore we make a runner for them and hide these tricky operations.

Besides, this library only supports tf/keras/scikit learn model. In our workflow, we expect the user to provide an onnx model and we will convert this onnx model to tf model (using onnx2tf package), and finally pass it to the 'kernel explainer' with a runner function.

As a result, we define a runner function in our code: https://github.com/dianna-ai/dianna/blob/40148ae9365f2444df578484adb8104b4bee3506/dianna/methods/kernelshap.py#L197-L213

I think it is very difficult for the kernelshap to support a function as input. We can make it work, but to me it seems this could take some time as we have to reshape the workflow again. I would suggest we raise an exception for it.

geek-yang avatar Apr 04 '22 19:04 geek-yang

Thanks for the explanation! I agree that an exception would be best then.

egpbos avatar Apr 06 '22 14:04 egpbos