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

Normalize input tensor names #59

Closed
wants to merge 6 commits into from
Closed

Conversation

nnshah1
Copy link
Contributor

@nnshah1 nnshah1 commented Oct 9, 2023

updating to make names lower case and change 'prompt' and 'text' to more generic:

'text_input', 'text_output'

rmccorm4
rmccorm4 previously approved these changes Oct 9, 2023
Copy link
Collaborator

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

LGTM, but need to sync with @nv-hwoo @matthewkotila for PA VLLM guide updates, and @pskiran1 for VLLM CI test updates.

L0_http/generate_endpoint_test.py uses a mock model so it can be updated separately, should be unaffected by this.

@nnshah1 nnshah1 requested a review from nv-hwoo October 9, 2023 21:26
matthewkotila added a commit to triton-inference-server/client that referenced this pull request Oct 9, 2023
@matthewkotila
Copy link
Contributor

@rmccorm4: LGTM, but need to sync with @nv-hwoo @matthewkotila for PA VLLM guide updates, and @pskiran1 for VLLM CI test updates.

L0_http/generate_endpoint_test.py uses a mock model so it can be updated separately, should be unaffected by this.

triton-inference-server/client#412 ready to go 👍🙏

nv-hwoo
nv-hwoo previously approved these changes Oct 9, 2023
Copy link

@nv-hwoo nv-hwoo left a comment

Choose a reason for hiding this comment

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

One nit comment but LGTM. I ran the changes with PA LLM guide and confirmed that it works 👍

@nnshah1 nnshah1 dismissed stale reviews from nv-hwoo and rmccorm4 via 94f2337 October 9, 2023 21:59
nv-hwoo
nv-hwoo previously approved these changes Oct 9, 2023
rmccorm4
rmccorm4 previously approved these changes Oct 9, 2023
tanmayv25
tanmayv25 previously approved these changes Oct 9, 2023
@nnshah1 nnshah1 dismissed stale reviews from tanmayv25, rmccorm4, and nv-hwoo via a83585f October 11, 2023 15:06
@nnshah1
Copy link
Contributor Author

nnshah1 commented Oct 11, 2023

@tanmayv25, @jbkyang-nvi let me know if we want to merge this or close it as the changes have merged into the backend.

@matthewkotila
Copy link
Contributor

What's still blocking this?

@matthewkotila
Copy link
Contributor

@nnshah1 @tanmayv25 @rmccorm4 @jbkyang-nvi Friendly ping on what is blocking this?

@nnshah1
Copy link
Contributor Author

nnshah1 commented Oct 23, 2023

@nnshah1 @tanmayv25 @rmccorm4 @jbkyang-nvi Friendly ping on what is blocking this?

Plan is to move this tutorial to reference the new backend and the changes for naming have already been made there. We wanted to stop making changes here to avoid duplication / sync issues.

@matthewkotila Let us know issues with that approach.

@nnshah1 nnshah1 closed this Nov 21, 2023
@nnshah1 nnshah1 deleted the nnshah-input-normalization branch November 21, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants