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

feat: add support fastchat http bindings #421

Merged
merged 1 commit into from
Sep 10, 2023

Conversation

leiwen83
Copy link
Contributor

  • feat: add support fastchat http bindings

Copy link
Member

@wsxiaoys wsxiaoys left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, a few comments

crates/http-api-bindings/src/fastchat.rs Show resolved Hide resolved
crates/tabby/src/serve/completions.rs Outdated Show resolved Hide resolved
crates/http-api-bindings/src/fastchat.rs Show resolved Hide resolved
Copy link
Member

@wsxiaoys wsxiaoys left a comment

Choose a reason for hiding this comment

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

A few nits

crates/tabby/src/serve/completions.rs Outdated Show resolved Hide resolved
crates/tabby/src/serve/completions.rs Show resolved Hide resolved
crates/tabby/src/serve/completions.rs Show resolved Hide resolved
crates/tabby/src/serve/completions.rs Outdated Show resolved Hide resolved
crates/http-api-bindings/src/fastchat.rs Show resolved Hide resolved
@leiwen83 leiwen83 requested a review from wsxiaoys September 10, 2023 12:04
@wsxiaoys wsxiaoys enabled auto-merge (squash) September 10, 2023 13:01
Copy link
Member

@wsxiaoys wsxiaoys left a comment

Choose a reason for hiding this comment

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

LGTM

@wsxiaoys
Copy link
Member

Hi @leiwen83 - Could you fix the CI checks, once fixed. it should be automatically merged.

auto-merge was automatically disabled September 10, 2023 13:28

Head branch was pushed to by a user without write access

* feat: add support fastchat http bindings

Signed-off-by: Lei Wen <wenlei03@qiyi.com>
@leiwen83
Copy link
Contributor Author

Hi @leiwen83 - Could you fix the CI checks, once fixed. it should be automatically merged.

CI is fixed

@wsxiaoys wsxiaoys merged commit e3c4a77 into TabbyML:main Sep 10, 2023
4 checks passed
@itlackey
Copy link

Hey @leiwen83 are you successful using FastChat with Tabby? If so, what model(s) are you using? I am trying to get it work and having some issues. Any advice you can lend is much appreciated.

@leiwen83
Copy link
Contributor Author

Hey @leiwen83 are you successful using FastChat with Tabby? If so, what model(s) are you using? I am trying to get it work and having some issues. Any advice you can lend is much appreciated.

Hi, there is return format structure change in fastchat mainstream, so a patch is needed to work with current mainstream fastchat #670

You may try that fix patch first.

@itlackey
Copy link

Thank you both for your help to make this work! I will pull the latest and give this a try this evening.

@itlackey
Copy link

itlackey commented Oct 30, 2023

So far the response from FastChat has been too slow for Tabby. It seems like it's due too flooding the API with requests. We may need some denounce logic I the http device and/or request cancelation (if possible).
If I switch to manual and send one completion request at a time it seems to work well enough, but with automatic it is pretty much unusable.

I'm running wizard 3b on an a770 with 16gb VRAM

Here is the compose file I am using with a local Tabby container built from the latest code on the main branch:
docker-compose.yml.txt

@leiwen83
Copy link
Contributor Author

So far the response from FastChat has been too slow for Tabby. It seems like it's due too flooding the API with requests. We may need some denounce logic I the http device and/or request cancelation (if possible). If I switch to manual and send one completion request at a time it seems to work well enough, but with automatic it is pretty much unusable.

I'm running wizard 3b on an a770 with 16gb VRAM

Here is the compose file I am using with a local Tabby container built from the latest code on the main branch: docker-compose.yml.txt

Yes, I also notice this slowness. For denounce logic, do you have any idea?

@itlackey
Copy link

I worked on it for a while last night but it doesn't seem to be working. I have never developed in Rust. So, I am not really sure if this is even on the right track, but here is my attempt:
https://github.com/itlackey/tabby/blob/feature/debounce/crates/http-api-bindings/src/fastchat.rs

@leiwen83 leiwen83 deleted the add_fastchat branch October 31, 2023 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants