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

Don't attempt batching with InstructLab's llama-cpp-python #358

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

bbrowning
Copy link
Contributor

Try harder to check for InstructLab's default llama-cpp-python during our batching check so that we can avoid throwing assertion errors on the server when we probe for batching support.

See instructlab/instructlab#1748 for user reports of this issue.

Try harder to check for InstructLab's default llama-cpp-python during
our batching check so that we can avoid throwing assertion errors on
the server when we probe for batching support.

See instructlab/instructlab#1748 for user reports of this issue.

Signed-off-by: Ben Browning <[email protected]>
@mergify mergify bot added testing Relates to testing ci-failure labels Nov 10, 2024
@bbrowning
Copy link
Contributor Author

There doesn't appear to be a robust way to identify llama-cpp-python as a HTTP client. However, InstructLab's most common happy path has users generating data against a llama-cpp-python managed by InstructLab and we CAN reliably identify that. We'll do that by looking for the API root welcome message and disable batching without actually attempting the batching request probe. That batching request attempt is what throws the assertion error in llama-cpp-python, and this change will avoid throwing those errors for anyone using ilab model serve to run their model.

@mergify mergify bot added ci-failure and removed ci-failure labels Nov 10, 2024
@bbrowning bbrowning requested a review from a team November 10, 2024 02:20
@mergify mergify bot removed the ci-failure label Nov 10, 2024
@bbrowning
Copy link
Contributor Author

We get lots of user reports via Slack and in this and other repos about the assertion error with llama-cpp-python. This gets rid of that, and will make for a much cleaner initial user experience. Otherwise, we often have users that think things are broken with data generation.

Copy link
Member

@aakankshaduggal aakankshaduggal left a comment

Choose a reason for hiding this comment

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

Thanks @bbrowning, lgtm!

@mergify mergify bot merged commit 119d309 into instructlab:main Nov 11, 2024
22 checks passed
@mergify mergify bot removed the one-approval label Nov 11, 2024
@bbrowning bbrowning deleted the no-llama-assertion-error branch November 11, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Relates to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants