-
Notifications
You must be signed in to change notification settings - Fork 1
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
tensorflow-serving rounds up results differently than upstream #86
Comments
My previous assumption about the root cause was not correct.
|
Thank you for reporting us your feedback! The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5216.
|
@orfeas-k do you know how the expected response data was generated in the first place? Are we sure those results are reproducible? |
Cherry-pick b9e9ac3 from #232. This adds: - Integrate ROCKs to charm and servers - Update `tf-serving` test results due to canonical/seldonio-rocks#86. We can see that `tensorflow` test that also uses the updated ROCK didn't need any change. Refs #224
It turns out those test data (both the request and the response) come from this 1.15.0 upstream example which is the same for 1.17.1. Those examples have been generated from this notebook.
Those were initially introduced in canonical/seldon-core-operator#152. However, while the request data is identical, the response data differs a bit in the sense that our test data have one or more floating digits. Here's a comparison between upstream example and our test response:
This difference could have to do with the format type. We see that the upstream response has a To your question @DnPlas, I can't conclude from the documentation how reproducible these results should be. They could be since we 've ran those tests multiple times without any issues. At the same time, there are issues upstream like this one tensorflow/serving#1856 (comment) where they mention that using Tensorflow doesn't guarantee deterministic results. |
After discussing with the team, we concluded that our tests should assert based on response status and structure since neither the use of ROCKs neither the charm should be able to break a seldonDeployment's results. Thus,we 'll close this issue and instead modify our tests assertion. |
Integration tests fail because ROCK rounds up results a bit differently which result in the following error:
This could be caused by the following warning that we observed also in #83
The text was updated successfully, but these errors were encountered: