-
Notifications
You must be signed in to change notification settings - Fork 56
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
[Serve] multiple submodels serve #298
Conversation
61007c2
to
7b89e05
Compare
3f00d5d
to
3b4e708
Compare
fb6204f
to
9ab5b39
Compare
38b60e3
to
01529bc
Compare
01529bc
to
06152f7
Compare
if with_test: | ||
f.write(f'bash -c "$cmd; sync" \n') | ||
# TODO: need a option to control whether to append or overwrite the output file | ||
# Now, it always appends to the output file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if there is an option available, users will be satisfied.
if self.command_line_mode: | ||
self.user_script = "flagscale/serve/run_vllm.py" | ||
elif isinstance(entrypoint, str) and entrypoint.endswith(".py"): | ||
self.user_script = entrypoint | ||
elif entrypoint is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we require users to provide the entrypoint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an option. Models pipeline can be built via user's entrypoint file or yaml config.
flagscale/serve/core/dag.py
Outdated
from flagscale.logger import logger | ||
|
||
|
||
class Builder: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builder for what? I believe the name should be more specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
flagscale/serve/core/dag.py
Outdated
def check_and_get_port(self, target_port=None, host="0.0.0.0"): | ||
""" | ||
Check if a specific port is free; if not, allocate a free port. | ||
:param target_port: The port number to check, default is None. | ||
:param host: The host address to check, default is "0.0.0.0". | ||
:return: A tuple (is_free, port), where `is_free` indicates if the target port is free, | ||
and `port` is the allocated port (target_port if free, or a new free port). | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code should be placed in the utilities so that it can be reused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Type
New Feature
Description
Given many submodules. Simply configure the keywords of the submodules, including resource allocation and interdependencies among the submodels.
Given many well-connected nodes. Simply configure the keywords of the nodes, including resource type and address messages.
Serve test case contains 2 steps: serve and call.