[Misc] Consolidate app and simulator #477
Conversation
zhangjyr
added
kind/cleanup
development/app/Dockerfile
Outdated
| COPY ./*.py /app/ | ||
|
|
||
| ENV MODEL_NAME=llama2-7b | ||
| ARG GPU_TYPE=disabled |
There was a problem hiding this comment.
minor: I'd rather to have two args, profiler enabled/disabled and gpu type.
if profile == enabled, gpu_types should exist.
This makes the code more readable for people who is not familiar with this feature
There was a problem hiding this comment.
I can add another bool switch here. Although the profiling necessity is the reason for separate images, this switch will also enable/disable the simulation feature. So, I will name the switch as "simulation."
There was a problem hiding this comment.
Things get a little complicated as:
- The vidur argparse overrides mock app argument parsing. So, no new argument can be added with the current implementation. This means even if I added a new ARG for Dockerfile, the mock app will still use one argument.
- The Dockerfile does not have complex condition logic. I currently use ENV to pass ARG to CMD; the difference between Dockerfile BUILD-ARGs and mock app arguments will be error-prone.
There was a problem hiding this comment.
then let's stick to one argument at this moment and rename to simulation, that looks good to me
|
Looks good to me. |
Change build arg from GPU_TYPE to SIMULATION
|
Seem all reviewers left feedbacks and all of them have been addressed, we can merge this change. |
* Merge simulator to app * Fix namespace. * Using model tokenizer if possible. Token is needed for gated model. Change build arg from GPU_TYPE to SIMULATION --------- Co-authored-by: Jingyuan Zhang <[email protected]>
Pull Request Description
We previously built a CPU-mocked app to facilitate fast prototyping and local development. This approach was convenient as it eliminated the need for GPU resources and simulated OpenAI-like interfaces and metrics effectively.
Recently, #430 introduced a separate app for simulation purposes. While it adds value, having separate apps for similar functionalities increases complexity and maintenance overhead.
This PR consolidates the simulator to the previous mock app. Since the simulator is GPU-specific, a separate base image is needed for the simulator of various GPUs. Currently, the default GPU is a100. Examples of config other GPUs are included.
Related Issues
Resolves: #456
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]: Corrections to existing functionality[CI]: Changes to build process or CI pipeline[Docs]: Updates or additions to documentation[API]: Modifications to aibrix's API or interface[CLI]: Changes or additions to the Command Line Interface[Misc]: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.