-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add explainer types in payload and return both LIME and SHAP by default #9
Add explainer types in payload and return both LIME and SHAP by default #9
Conversation
@@ -50,6 +55,9 @@ public class ExplainerV1Endpoint { | |||
@Consumes(MediaType.APPLICATION_JSON) | |||
public Response explain(@PathParam("modelName") String modelName, KServeV1RequestPayload data) | |||
throws ExecutionException, InterruptedException { | |||
// SaliencyResults shapResults = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented lines
@christinaexyou could you change the main README.md too to reflect this change? Can be in a separate PR if you prefer. |
} else if (explainerType == ExplainerType.SHAP) { | ||
// SHAP only | ||
final SaliencyResults results = explainer.explainAsync(prediction, provider).get(); | ||
response = SaliencyExplanationResponse.fromSaliencyResult(ExplainerType.SHAP, ts(results, ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo? This is making the build fail.
// Handle different explainer types | ||
if (explainerType == ExplainerType.LIME) { | ||
// LIME only | ||
final SaliencyResults results = explainer.explainAsync(prediction, provider).get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both requests should be done async and merged only when both are complete.
d9368c4
to
177bc85
Compare
Addresses issue #8 by making the following changes:
src/main/java/org/kie/trustyai/ConfigService.java
to return both LIME and SHAPsrc/main/java/org/kie/trustyai/ExplainerV1Endpoint.java
&src/main/java/org/kie/trustyai/payloads/SaliencyExplanationResponse.java
with functionality to get both LIME and SHAP or either and adds the explainer type in the infoCurrently, if the explainer type is null then we return both LIME and SHAP but let me know if I should create a new explainer type to be more explicit.