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

[server] Add model argument to server cli #1584

Merged
merged 3 commits into from
Feb 7, 2024
Merged

[server] Add model argument to server cli #1584

merged 3 commits into from
Feb 7, 2024

Conversation

dsikka
Copy link
Contributor

@dsikka dsikka commented Feb 6, 2024

Summary:

deepsparse.server \
 "zoo:llama2-7b-ultrachat200k_llama2_pretrain-pruned50_quantized" \
 --integration openai

PR Update

This PR allows us to get the following cli command:

deepsparse.server --integration openai "hf:mgoin/TinyStories-1M-ds"

We can run the following commands with the current set-up:

deepsparse.server --task text_generation --model_path "hf:mgoin/TinyStories-1M-ds"
deepsparse.server --model_path "hf:mgoin/TinyStories-1M-ds" --integration openai
deepsparse.server --task text_generation "hf:mgoin/TinyStories-1M-ds"
deepsparse.server --intergration openai --task text_generation  "hf:mgoin/TinyStories-1M-ds"
deepsparse.server --config_file ~/debugging/sample_config.yaml

Caveats (@bfineran):

  • This also does not let us run the cli command by providing the model path first (i.e will not directly follow what is shown in the ticket/UX docs; the integration or task option would have to be first)

Shoutout to @rahul-tuli for his click knowledge and help

@dsikka dsikka requested review from rahul-tuli and bfineran February 6, 2024 14:43
bfineran
bfineran previously approved these changes Feb 6, 2024
@mgoin
Copy link
Member

mgoin commented Feb 6, 2024

is this a breaking change if we were using --model_path first? this seems fairly important for all server flows and we should hopefully be able to deprecate rather than removing

@dsikka
Copy link
Contributor Author

dsikka commented Feb 6, 2024

@mgoin yes. If we make model_path an argument to support the UX docs, we can't use it as an option as well ---model_path

We could add a separate model path entry point but click can't support both AFAIK

@bfineran
Copy link
Contributor

bfineran commented Feb 6, 2024

@mgoin yes. If we make model_path an argument to support the UX docs, we can't use it as an option as well ---model_path

We could add a separate model path entry point but click can't support both AFAIK

@dsikka per @markurtz lets add a model_path kwarg back in (can rename the positional arg to something else) and allow it to override the positional if given. (Would need to make the positional arg optional in this case I guess)

@dsikka dsikka changed the title [server] Update model path to be an argument [server] Add model argument to server cli Feb 7, 2024
@dsikka dsikka merged commit 2e33e67 into main Feb 7, 2024
13 checks passed
@dsikka dsikka deleted the update_server_init branch February 7, 2024 19:24
dsikka added a commit that referenced this pull request Feb 7, 2024
* update model path to be an argument; remove unused openai command pathway

* add model path arg and option
bfineran pushed a commit that referenced this pull request Feb 7, 2024
* update model path to be an argument; remove unused openai command pathway

* add model path arg and option
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.

3 participants