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

Huggingface configurations refactoring #1283

Merged
merged 2 commits into from
Nov 8, 2023
Merged

Huggingface configurations refactoring #1283

merged 2 commits into from
Nov 8, 2023

Conversation

sindhuvahinis
Copy link
Contributor

@sindhuvahinis sindhuvahinis commented Nov 7, 2023

@sindhuvahinis sindhuvahinis force-pushed the hf branch 3 times, most recently from b59f114 to 953aee7 Compare November 7, 2023 21:26
@sindhuvahinis sindhuvahinis marked this pull request as ready for review November 7, 2023 23:00
@sindhuvahinis sindhuvahinis requested review from zachgk, frankfliu and a team as code owners November 7, 2023 23:00
# device map is not required for lmi dist and
if properties['rolling_batch'] == RollingBatchEnum.lmidist or \
properties['rolling_batch'] == RollingBatchEnum.vllm:
return properties
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xyang16 @KexinFeng @lanking520
I am not validating whether device_map exists for lmi dist and vllm. And also not inserting load_in_8bit in kwargs. Kindly check if this is okay,

Copy link
Contributor

Choose a reason for hiding this comment

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

device map is not important. this is only applies to scheduler and HF Accelerate path

bitsandbytes8 = 'bitsandbytes8'

# supported by vllm
awq = 'awq'
Copy link
Contributor Author

@sindhuvahinis sindhuvahinis Nov 7, 2023

Choose a reason for hiding this comment

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

Kindly verify whether does it cover all supported quantization methods. lmi-dist has tests for gptq. vllm does not have test case for awq in our pipeline. Let me add it.

@xyang16 @lanking520

Copy link
Contributor

Choose a reason for hiding this comment

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

there is no awq test for vLLM, feel free to add it

@sindhuvahinis sindhuvahinis merged commit 771a678 into master Nov 8, 2023
8 checks passed
@sindhuvahinis sindhuvahinis deleted the hf branch November 8, 2023 06:01
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.

2 participants