Add inference notebook to the workflow #2
Conversation
04_model_inference.ipynb
Outdated
| "outputs": [], | ||
| "source": [ | ||
| "# endpoint from the seldon deployment\n", | ||
| "base_url = \"http://dep1-route-aiops-tools-workshop.apps.smaug.na.operate-first.cloud/predict\"" |
There was a problem hiding this comment.
hey seems like this notebook is still pointing to the deployed route. Currently we do not deploy the model as a part of the workflow yet. Can you have this notebook get inference from the model stored on the S3 bucket instead. That is what we are currently doing as a part of the workflow.
There was a problem hiding this comment.
@aakankshaduggal seems like the notebook expects a deployed seldon endpoint which we do not have the functionality for as yet. can we use the last notebook to validate the model that we store on S3?
|
@aakankshaduggal please let me know when this is ready for re-review :) |
|
@oindrillac Please feel free to review, just need to get rid of the warnings. |
|
@oindrillac all set now! |
04_model_inference.ipynb
Outdated
| "## In this step\n", | ||
| "\n", | ||
| "\n", | ||
| "The purpose of this notebook is to check whether this service is running as intended, and more specifically to ensure that the model performance is what we expect it to be. So here, we will use the test set from the aforementioned notebook as the query payload for the service, and then verify that the return values are the same as those obtained during training/testing locally.\n", |
There was a problem hiding this comment.
The model is not a service. Lets say that we are sending the payload to the saved model on S3 to see how the model is performing on the test data ( a segment of past reviews )
04_model_inference.ipynb
Outdated
| } | ||
| ], | ||
| "source": [ | ||
| "sample_payload" |
There was a problem hiding this comment.
can you remove this extra consecutive print?
04_model_inference.ipynb
Outdated
| "source": [ | ||
| "# Conclusion\n", | ||
| "\n", | ||
| "This notebook shows how raw PR data can be sent to the deployed Seldon service to get time-to-merge predictions. Additionally, we see that the evaluation scores in the classification report match the ones we saw in the training notebook. So, great, looks like our inference service and model are working as expected, and are ready to predict some times to merge for GitHub PRs! " |
There was a problem hiding this comment.
Lets modify the text here to reflect our approach in this notebook. We arent interacting with a service any longer
|
@oindrillac thanks for the comments, fixed that |
04_model_inference.ipynb
Outdated
| "/opt/app-root/lib64/python3.8/site-packages/sklearn/metrics/_classification.py:1334: UndefinedMetricWarning: Precision and F-score are ill-defined and being set to 0.0 in labels with no predicted samples. Use `zero_division` parameter to control this behavior.\n", | ||
| " _warn_prf(average, modifier, msg_start, len(result))\n", | ||
| "/opt/app-root/lib64/python3.8/site-packages/sklearn/metrics/_classification.py:1334: UndefinedMetricWarning: Recall and F-score are ill-defined and being set to 0.0 in labels with no true samples. Use `zero_division` parameter to control this behavior.\n", | ||
| " _warn_prf(average, modifier, msg_start, len(result))\n" |
There was a problem hiding this comment.
Can we also catch the UndefinedMetricWarnings so these are also suppressed?
There was a problem hiding this comment.
Yes, suppressed those. But usually, it's recommended to keep the zero division error so the user is aware of that situation. Lmk what you think
Addresses aicoe-aiops/ocp-ci-analysis#602