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

[ci] Updating lmi-dist ci tests for rubikon-engine #1651

Merged
merged 1 commit into from
Mar 23, 2024

Conversation

maaquib
Copy link
Contributor

@maaquib maaquib commented Mar 21, 2024

Description

Following changes have been made

  • Removes bitsandbytes test case, since we no longer support it
  • Adds speculative decoding test case
  • Adds tests for Gemma-7B and Santacoder
  • If this change is a backward incompatible change, why must this change be made? - We're dropping support for bitsandbytes quantization
  • Interesting edge cases to note here - N/A

Copy link
Contributor

@lanking520 lanking520 left a comment

Choose a reason for hiding this comment

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

could you also remove the speculative decoding test in vLLM?

.github/workflows/rolling_batch_integration.yml Outdated Show resolved Hide resolved
@maaquib maaquib force-pushed the rubikon-engine-ci branch 3 times, most recently from fabc9d0 to 1ff1005 Compare March 21, 2024 21:30
@maaquib maaquib force-pushed the rubikon-engine-ci branch from 1ff1005 to d339ef6 Compare March 21, 2024 22:57
@maaquib maaquib force-pushed the rubikon-engine-ci branch from d339ef6 to be31ff6 Compare March 21, 2024 23:10
@maaquib
Copy link
Contributor Author

maaquib commented Mar 21, 2024

@maaquib maaquib marked this pull request as ready for review March 21, 2024 23:53
@maaquib maaquib requested review from zachgk, frankfliu and a team as code owners March 21, 2024 23:53
Copy link
Contributor

@davidthomas426 davidthomas426 left a comment

Choose a reason for hiding this comment

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

Is it ok for our CI tests to use HF model ids directly rather than s3 links?

Other than that, LGTM.

@lanking520
Copy link
Contributor

lanking520 commented Mar 22, 2024

Is it ok for our CI tests to use HF model ids directly rather than s3 links?

Other than that, LGTM.

There is a problem with HF id. Firstly it is not very stable. Secondly, sometimes the download speed is very slow and cause timeout (flaky tests)

@lanking520
Copy link
Contributor

@maaquib could you also just replace the lmi-dist handler with the new one in this PR, so we can just cover in CI testing?

@maaquib maaquib force-pushed the rubikon-engine-ci branch 3 times, most recently from fba9e9d to dac46f0 Compare March 22, 2024 02:48
@lanking520
Copy link
Contributor

could you also add tests for vLLM as well, just to serve as baseline?

@maaquib maaquib force-pushed the rubikon-engine-ci branch 2 times, most recently from a23f83c to a890bd6 Compare March 22, 2024 15:44
@maaquib
Copy link
Contributor Author

maaquib commented Mar 22, 2024

could you also add tests for vLLM as well, just to serve as baseline?

Added gemma and starcoder2 to vllm

@maaquib maaquib force-pushed the rubikon-engine-ci branch 5 times, most recently from d33c72e to f69e891 Compare March 22, 2024 17:09
@maaquib maaquib force-pushed the rubikon-engine-ci branch from f69e891 to f7dff75 Compare March 22, 2024 17:17
@maaquib maaquib force-pushed the rubikon-engine-ci branch from f7dff75 to c63330f Compare March 22, 2024 19:05
@maaquib
Copy link
Contributor Author

maaquib commented Mar 22, 2024

@maaquib maaquib force-pushed the rubikon-engine-ci branch 5 times, most recently from 5a1cbbf to 5adc189 Compare March 23, 2024 02:17
@maaquib maaquib force-pushed the rubikon-engine-ci branch from 5adc189 to 45dca51 Compare March 23, 2024 04:46
@lanking520 lanking520 merged commit 0747d91 into master Mar 23, 2024
31 of 38 checks passed
@lanking520 lanking520 deleted the rubikon-engine-ci branch March 23, 2024 06:32
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