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

[Frontend]-config-cli-args #7737

Merged
merged 34 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0d304d7
[Frontend]-config-cli-args
KaunilD Aug 21, 2024
f36dc39
Update vllm/scripts.py
KaunilD Aug 21, 2024
2bca2fa
[Frontend]-config-cli-args
KaunilD Aug 21, 2024
ab570d1
[Frontend]-config-cli-args updated docs
KaunilD Aug 21, 2024
7bfc6cb
[Frontend]-config-cli-args updated docs
KaunilD Aug 22, 2024
1779536
Update docs/source/serving/openai_compatible_server.md
KaunilD Aug 23, 2024
ff93954
[Frontend]-config-cli-args integrated configargparse
Aug 23, 2024
a9492c4
[Frontend]-config-cli-args removed comfig.yaml
Aug 23, 2024
05164e0
[Frontend]-config-cli-args removed comfig.yaml
Aug 23, 2024
7014a4e
[Frontend]-config-cli-args renamed function signature
Aug 23, 2024
63413aa
[Frontend]-config-cli-args formattting
Aug 23, 2024
4d6f930
[Frontend]-config-cli-args added native support
Aug 26, 2024
0d41c4c
[Frontend]-config-cli-args added native support
Aug 26, 2024
8d84671
[Frontend]-config-cli-args added native support
Aug 26, 2024
c5af059
[Frontend]-config-cli-args added native support
Aug 26, 2024
f6529e3
[Frontend]-config-cli-args added native support
Aug 26, 2024
6e1fe11
[Frontend]-config-cli-args added tests
Aug 26, 2024
175a0d5
[Frontend]-config-cli-args added tests
Aug 26, 2024
7c06e17
[Frontend]-config-cli-args added tests
Aug 26, 2024
98208b3
[Frontend]-config-cli-args added tests
Aug 26, 2024
3d72a70
[Frontend]-config-cli-args updated tests
KaunilD Aug 27, 2024
10054a4
[Frontend]-config-cli-args updated tests
KaunilD Aug 27, 2024
b87593b
[Frontend]-config-cli-args updated tests
KaunilD Aug 27, 2024
56a7054
[Frontend]-config-cli-args thinned diff
KaunilD Aug 27, 2024
2c7df07
[Frontend]-config-cli-args thinned diff
KaunilD Aug 27, 2024
7b77458
Merge branch 'main' into kaunild/frontend/config-cli-args
KaunilD Aug 27, 2024
a5b1a3a
[Frontend]-config-cli-args updated tests
KaunilD Aug 27, 2024
d189970
Update vllm/utils.py
KaunilD Aug 30, 2024
ae178fb
Update vllm/utils.py
KaunilD Aug 30, 2024
0c1b302
Update docs/source/serving/openai_compatible_server.md
KaunilD Aug 30, 2024
743aee5
Merge branch 'main' into kaunild/frontend/config-cli-args
KaunilD Aug 30, 2024
295f675
[Frontend]-config-cli-args
Aug 30, 2024
960b047
Merge branch 'kaunild/frontend/config-cli-args' of github.com:KaunilD…
Aug 30, 2024
b6f130d
[Frontend]-config-cli-args
Aug 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions docs/source/serving/openai_compatible_server.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,29 @@ directory [here](https://github.com/vllm-project/vllm/tree/main/examples/)
:prog: vllm serve
```

### Config file

You may also supply these CLI args using a config file. For example:

```yaml
# config.yaml

host: "127.0.0.1"
port: 6379
uvicorn-log-level: "info"
```

```bash
$ vllm serve SOME_MODEL --config config.yaml
```

---
**NOTE**

In case an argument is supplied using CLI and the config file, the value from the CLI will take precedence.

---

DarkLight1337 marked this conversation as resolved.
Show resolved Hide resolved
## Tool calling in the chat completion API
vLLM supports only named function calling in the chat completion API. The `tool_choice` options `auto` and `required` are **not yet supported** but on the roadmap.

Expand Down
57 changes: 56 additions & 1 deletion vllm/scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@
import os
import signal
import sys
from typing import List, Optional
from typing import Dict, List, Optional, Union

import yaml
from openai import OpenAI
from openai.types.chat import ChatCompletionMessageParam

from vllm.engine.arg_utils import EngineArgs
from vllm.entrypoints.openai.api_server import run_server
from vllm.entrypoints.openai.cli_args import make_arg_parser
from vllm.logger import init_logger
from vllm.utils import FlexibleArgumentParser

logger = init_logger(__name__)


def register_signal_handlers():

Expand All @@ -24,7 +28,49 @@ def signal_handler(sig, frame):
signal.signal(signal.SIGTSTP, signal_handler)


def _merge_args_and_config(args: argparse.Namespace):
"""
merge args from cli and config file supplied in the cli.
If an argument is present in cli and args then choose args value
over config file.

example:
# config.yaml
port: 1231

# invoke server
$ vllm serve --config ../config.yaml --port 3122

# selected port = 3122
"""
assert args.config, 'No config file specified.'

# only expecting a flat dictionary of atomic types
config: Dict[str, Union[int, str]] = {}

try:
with open(args.config, 'r') as config_file:
config = yaml.safe_load(config_file)
except Exception as ex:
logger.error("Unable to read the config file at %s", args.config)
logger.error(ex)

for key, value in config.items():
if hasattr(args, key):
Copy link
Member

Choose a reason for hiding this comment

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

how does this deal with options that have default values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tested. same behaviour. default args are populated in the args.Namespace before it enters the method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*same ebhaviour as in cli takes presidence, if specified in config file as well. and default in case no where speficied..

Copy link
Member

Choose a reason for hiding this comment

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

I mean, even if I don't specify -tp 1, args.tensor_parallel will have value. in this case, the default cli value takes precedence over config file, which is not desirable.

Copy link
Contributor Author

@KaunilD KaunilD Aug 23, 2024

Choose a reason for hiding this comment

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

please bear with me @youkaichao

i see so if i understand correctly.. ideal behavior would be that; the user supplies ALLthe args in config file, if --config has been used, even the default ones? In a manner of speaking the config file will always have ~105 lines in them (current population of namespace)..
so for example:

Ex1

#config.yaml

port: 1234
$ vllm serve "fabebook/opt-12b" --config config.yaml
$ [ERROR] Please specify all args in config file while using --config 

Ex2

#config.yaml

port: 1234
... 104 more..
$ vllm serve "fabebook/opt-12b" --config config.yaml --port 4
$ [INFO] port = 4

Ex3

$ vllm serve "fabebook/opt-12b" 
$ [INFO] port = 8000 (default)

Copy link
Member

Choose a reason for hiding this comment

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

Looks like a misunderstanding.

I'm trying to understand, how do you know if getattr(args, key) is a default value or a value from commandline?

Copy link
Contributor Author

@KaunilD KaunilD Aug 23, 2024

Choose a reason for hiding this comment

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

oooh got it!
I understand it now. we will have to handle other cases as well with validating if arguments in config file have membership with the vllm namespace, shorthand etc and c..
with this in mind i think its prudent for us to use configargparse which inherits from argparse.
this will keep the diff small with largest ROI as well. moreover, it doesnt neeed anything more than what is already in vllm's requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PS:
I have also tested the the scratch implementation using sys.argv check for default membership and then overriding args appropirately. we can go ahead with that too if its too much of a leap in faith to use configargparse module..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@youkaichao hope you are doing well.

Unfortunately with configargparse the tests are not passing. The server is not able to start quickly enough for the api tests to pass..

I will go ahead with the sys.argv implementation since its "lightweight" and more tailor made for our purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@youkaichao i feel good about this. Please let me know what do you think.

logger.info("Argument %s is specified via config and commandline.",
key)
logger.info("Selecting the %s=%s from commandline.", key,
getattr(args, key))
continue

setattr(args, key, value)


def serve(args: argparse.Namespace) -> None:

if args.config:
_merge_args_and_config(args)

# The default value of `--model`
if args.model != EngineArgs.model:
raise ValueError(
Expand Down Expand Up @@ -125,6 +171,15 @@ def main():
serve_parser.add_argument("model_tag",
type=str,
help="The model tag to serve")
serve_parser.add_argument(
"--config",
type=str,
required=False,
default='',
help="Read CLI options from a config file."
"Must be a YAML with the following options:"
"https://docs.vllm.ai/en/latest/serving/openai_compatible_server.html#command-line-arguments-for-the-server"
)
serve_parser = make_arg_parser(serve_parser)
serve_parser.set_defaults(dispatch_function=serve)

Expand Down
Loading