Skip to content

[tests] enable bnb tests on xpu - #36233

Merged
ydshieh merged 12 commits into
huggingface:mainfrom
faaany:device-fix
Feb 24, 2025
Merged

[tests] enable bnb tests on xpu#36233
ydshieh merged 12 commits into
huggingface:mainfrom
faaany:device-fix

Conversation

@faaany

@faaany faaany commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

As the title suggests, this PR enables all bnb-related tests on XPU.

@ydshieh

@faaany
faaany marked this pull request as ready for review February 21, 2025 05:20
@faaany faaany changed the title [tests] fix more device bugs [tests] enable bnb tests on xpu Feb 21, 2025

@ydshieh ydshieh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are those tests now passing on XPU?

Comment thread tests/quantization/bnb/test_4bit.py Outdated
encoded_input = self.tokenizer(self.input_text, return_tensors="pt")
output_sequences = self.model_4bit.generate(
input_ids=encoded_input["input_ids"].to(torch_device), max_new_tokens=10
input_ids=encoded_input["input_ids"].to(self.model_4bit.device), max_new_tokens=10

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see several similar changes in this PR? Just wondering if such changes are necessary? (happy to keep them though)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, they are necessary, because torch_device is "xpu", which defaults to "xpu:0", but the model is loaded with "device_map" which is not always "xpu:0". The same error also happens on CUDA. If users only have 1 GPU card, it will be fine, but for machines with multiple cards, this will raise an error.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can confirm. I made some similar changes in #35172 but looks like I missed some. Thanks @faaany!

@faaany

faaany commented Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

Are those tests now passing on XPU?

yes, they are passing on XPU.

@ydshieh

ydshieh commented Feb 24, 2025

Copy link
Copy Markdown
Collaborator

Great, I will merge as @matthewdouglas also approved which gives me a lot more confidence.

Thank you for taking care XPU with 🤗 !

@ydshieh
ydshieh merged commit 4dbf17c into huggingface:main Feb 24, 2025
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