-
Notifications
You must be signed in to change notification settings - Fork 23
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
update README, requirements.txt #23
Conversation
Signed-off-by: James Kunstle <[email protected]>
@@ -10,3 +10,4 @@ accelerate | |||
pandas | |||
pandas-stubs | |||
lm-eval>=0.4.2 | |||
instructlab |
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 doesn't seem right. Why is it needed here?
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.
Yeah we can't have this here - it's a circular dependency
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.
You can put it in requirements-dev.txt to get it installed for unit testing. You don't want it in your prod requirements.txt though.
This bit me over here: instructlab/sdg#28
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.
++ good feedback, this was something I needed for testing.
@@ -30,6 +30,9 @@ pip install vllm | |||
python -m vllm.entrypoints.openai.api_server --model instructlab/granite-7b-lab --tensor-parallel-size 1 | |||
``` | |||
|
|||
> Note: You may need to be specific about which Python version you use for your virtual environment. | |||
> e.g. `python3.11 -m venv venv` is supported. |
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.
Noting I submitted a PR to update cloud-instance as well: instructlab/instructlab#1487
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.
see comment on requirements.txt
closing because I don't think the addition is relevant anymore |
Fixes a couple of issues that I ran into when trying to run tests.