Skip to content
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 mismatch key vs name gpu io test #5143

Merged
merged 5 commits into from
Dec 8, 2022
Merged

Conversation

kthui
Copy link
Contributor

@kthui kthui commented Dec 7, 2022

The current L0_tf_gpu_io test relies on well created TF savedmodels, which the I/O tensors key and name will always match, but this is not the case in the real world. The added model and test is derived from the same "qa_identity_model_repository/savedmodel_nobatch_zero_1_float32" but deliberately made the key and name to not match for testing the change.

Related PR: triton-inference-server/tensorflow_backend#84

@kthui

This comment was marked as outdated.

qa/L0_tf_gpu_io/test.sh Outdated Show resolved Hide resolved
qa/L0_tf_gpu_io/test.sh Outdated Show resolved Hide resolved
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

name: "mismatch_key_name"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name it differently, IIUC, it is still an valid model, just that its signature def expose the I/O in names different from the tensor names in the TF graph.

I think the model created from gen_tag_sigdef.py has this characteristic so you should be able to re-use that for this test case. CC @jbkyang-nvi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Updated.

@kthui
Copy link
Contributor Author

kthui commented Dec 7, 2022

With the sig_tag0 model, when the test is run without the TF change, the server will fail to start:

+----------+---------+---------------------------------------------------------------------------------------------------------------------+
| Model    | Version | Status                                                                                                              |
+----------+---------+---------------------------------------------------------------------------------------------------------------------+
| sig_tag0 | 1       | UNAVAILABLE: Internal: Tensor INPUT:0, specified in either feed_devices or fetch_devices was not found in the Graph |
+----------+---------+---------------------------------------------------------------------------------------------------------------------+

@kthui kthui requested review from GuanLuo and nv-kmcgill53 and removed request for nv-kmcgill53 December 7, 2022 03:03
@kthui
Copy link
Contributor Author

kthui commented Dec 7, 2022

CC @nv-kmcgill53, PR for TF gpu_io MakeCallable fix.

exit 1
fi
kill $SERVER_PID
wait $SERVER_PID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Send request to test if the model is working.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And check the response

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Updated.

@kthui kthui requested a review from GuanLuo December 8, 2022 01:46
@kthui kthui force-pushed the jacky-fix-make-callable-name branch from c648b81 to 2ad7532 Compare December 8, 2022 18:29
@kthui kthui force-pushed the jacky-fix-make-callable-name branch from 2ad7532 to 697dc6b Compare December 8, 2022 18:30
@kthui
Copy link
Contributor Author

kthui commented Dec 8, 2022

Rebased. Changes are from c0911a5 to 697dc6b

@kthui kthui merged commit 93ceefa into main Dec 8, 2022
@kthui kthui deleted the jacky-fix-make-callable-name branch May 12, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants