Skip to content
This repository was archived by the owner on Sep 4, 2025. It is now read-only.

feat: add TGIS CLI commands#92

Closed
rafvasq wants to merge 3 commits intoopendatahub-io:mainfrom
rafvasq:cli-feat
Closed

feat: add TGIS CLI commands#92
rafvasq wants to merge 3 commits intoopendatahub-io:mainfrom
rafvasq:cli-feat

Conversation

@rafvasq
Copy link
Copy Markdown

@rafvasq rafvasq commented Jul 16, 2024 •
edited by dtrifiro
Loading

Related to

vllm-project#5090
Previously, vllm-project#4167

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 16, 2024

Hi @rafvasq. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@prashantgupta24
Copy link
Copy Markdown

prashantgupta24 commented Jul 16, 2024

Looks good @rafvasq ! Once we sync with upstream, we can get this in!

@prashantgupta24
Copy link
Copy Markdown

prashantgupta24 commented Jul 16, 2024

/retest

@rafvasq rafvasq force-pushed the cli-feat branch 2 times, most recently from ab66c57 to a38524f Compare July 17, 2024 18:54
@rafvasq @prashantgupta24
Add tgis tools
3b05677 
Signed-off-by: Rafael Vasquez <rafvasq21@gmail.com>
Co-authored-by: Prashant Gupta <prashantgupta@us.ibm.com>
prashantgupta24
prashantgupta24 reviewed Jul 17, 2024
View reviewed changes
Comment thread vllm/entrypoints/openai/api_server.py
Copy link
Copy Markdown

@prashantgupta24 prashantgupta24 Jul 17, 2024

Choose a reason for hiding this comment

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

we don't need changes to this file anymore, the __main__.py file handles starting both servers now!

prashantgupta24
prashantgupta24 reviewed Jul 17, 2024
View reviewed changes
Comment thread vllm/tgis_utils/hub.py
return to_remove


def convert_file(pt_file: Path, sf_file: Path, discard_names: List[str]):
Copy link
Copy Markdown

@prashantgupta24 prashantgupta24 Jul 17, 2024

Choose a reason for hiding this comment

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

Worth checking if this function is the same thing? - https://github.com/vllm-project/vllm/blob/main/vllm/model_executor/model_loader/weight_utils.py#L81

prashantgupta24
prashantgupta24 reviewed Jul 17, 2024
View reviewed changes
Comment thread vllm/scripts.py Outdated
print(output)


def download_weights(
Copy link
Copy Markdown

@prashantgupta24 prashantgupta24 Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if moving all these functions to a file that's not going to be changed by upstream vllm will help in future merge conflicts (Basically keeping our changes to a minimum to files that might be changed in upstream)

@prashantgupta24 prashantgupta24 mentioned this pull request Jul 17, 2024
Closed
@github-actions github-actions Bot changed the title feat: CLI commands Sync with upstream@v0.5.2-35-g1c27d25f Jul 18, 2024
@rafvasq rafvasq changed the title Sync with upstream@v0.5.2-35-g1c27d25f feat: add TGIS CLI commands Jul 18, 2024
@github-actions github-actions Bot changed the title feat: add TGIS CLI commands Sync with upstream@v0.5.2-53-g6366efc6 Jul 19, 2024
@dtrifiro dtrifiro changed the title Sync with upstream@v0.5.2-53-g6366efc6 feat: add TGIS CLI commands Jul 19, 2024
@dtrifiro dtrifiro self-requested a review July 19, 2024 13:05
dtrifiro
dtrifiro reviewed Jul 19, 2024
View reviewed changes
Copy link
Copy Markdown

@dtrifiro dtrifiro left a comment

Choose a reason for hiding this comment

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

Shouldn't all of this go to https://github.com/opendatahub-io/vllm-tgis-adapter ? @prashantgupta24

@prashantgupta24
Copy link
Copy Markdown

prashantgupta24 commented Jul 19, 2024

@dtrifiro I had debated that, but yeah that might make the future upstream conflict issue go away if we do that. Might need some changes to make it work with the adapter

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 19, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rafvasq
Once this PR has been reviewed and has the lgtm label, please assign terrytangyuan for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@prashantgupta24
Copy link
Copy Markdown

prashantgupta24 commented Jul 19, 2024

@rafvasq can you try incorporating this in the vllm-tgis-adapter that Daniele linked above? Apologies for moving this PR around so much, but a lot of things are changing rapidly

@rafvasq
Separate extra commands
a9f34f2 
Signed-off-by: Rafael Vasquez <rafvasq21@gmail.com>
@rafvasq
Copy link
Copy Markdown
Author

rafvasq commented Jul 19, 2024

No problem @prashantgupta24!

@rafvasq rafvasq marked this pull request as draft July 19, 2024 20:10
@github-actions github-actions Bot changed the title feat: add TGIS CLI commands Sync with upstream@v0.5.2-67-g9042d683 Jul 20, 2024
@github-actions github-actions Bot changed the title Sync with upstream@v0.5.2-67-g9042d683 Sync with upstream@v0.5.2-73-gd7f4178d Jul 21, 2024
@github-actions github-actions Bot changed the title Sync with upstream@v0.5.2-73-gd7f4178d Sync with upstream@v0.5.2-79-g42de2cef Jul 22, 2024
@github-actions github-actions Bot changed the title Sync with upstream@v0.5.2-79-g42de2cef Sync with upstream@v0.5.2-93-gc051bfe4 Jul 23, 2024
@dtrifiro
Copy link
Copy Markdown

dtrifiro commented Jul 23, 2024

Closing as this belongs in vllm-tgis-adapter.

@dtrifiro dtrifiro closed this Jul 23, 2024
@dtrifiro dtrifiro changed the title Sync with upstream@v0.5.2-93-gc051bfe4 feat: add TGIS CLI commands Jul 23, 2024
@rafvasq rafvasq mentioned this pull request Jul 26, 2024
Merged
prarit pushed a commit to prarit/vllm that referenced this pull request Oct 18, 2024
@charlifu @gshtras
Reduce csv writes (opendatahub-io#92)
37c5acc 
* Reading the shapes csv only once and writing only if a new shape is deicovered

* fix lint

---------

Co-authored-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@heyselbi heyselbi Awaiting requested review from heyselbi

@vaibhavjainwiz vaibhavjainwiz Awaiting requested review from vaibhavjainwiz

2 more reviewers

@prashantgupta24 prashantgupta24 prashantgupta24 left review comments

@dtrifiro dtrifiro dtrifiro left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

do-not-merge/work-in-progress ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rafvasq @prashantgupta24 @dtrifiro @openshift-merge-robot