这是indexloc提供的服务，不要输入任何密码
Skip to content

Fix compare results #2473

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 3, 2025
Merged

Fix compare results #2473

merged 2 commits into from
Apr 3, 2025

Conversation

@jiqing-feng
Copy link
Contributor

@jiqing-feng jiqing-feng commented Apr 1, 2025 •
edited
Loading

Hi @BenjaminBossan . I tested tests/bnb/test_bnb_regression.py on A100 and XPU with the latest multi-backend-refactor branch of bitsandbytes. The results might be slightly different as bnb updates. Please review this PR. Thanks!

@jiqing-feng jiqing-feng changed the title Test 2 Fix compare results Apr 2, 2025
@jiqing-feng jiqing-feng marked this pull request as ready for review April 2, 2025 01:26
@BenjaminBossan
Copy link
Member

BenjaminBossan commented Apr 2, 2025

Thanks for the PR. I don't think we should change the expected_bytes, otherwise the test would start failing on other setups, right?

Also, just to give more context: These are all pure bitsandbytes tests, they don't really have anything to do with PEFT. We added them here because bitsandbytes didn't have their own CI at the time and we wanted to help out. This will soon change and we will probably phase out the bnb tests in PEFT. Therefore, I wonder if we should not just leave the file as it is and you can skip this test on your internal XPU CI. WDYT?

@jiqing-feng
Copy link
Contributor Author

jiqing-feng commented Apr 2, 2025

Hi @BenjaminBossan . Yes, it should be easier for me. Please review the new changes. Thanks!

@BenjaminBossan
Copy link
Member

BenjaminBossan commented Apr 2, 2025

We had some issues with the CI that should now be fixed, could you please merge with/rebase on the latest main?

@jiqing-feng
Copy link
Contributor Author

jiqing-feng commented Apr 3, 2025

We had some issues with the CI that should now be fixed, could you please merge with/rebase on the latest main?

Done. Already merged the main branch.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 3, 2025

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

BenjaminBossan
BenjaminBossan approved these changes Apr 3, 2025
View reviewed changes
Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

@BenjaminBossan BenjaminBossan merged commit 1cf886b into huggingface:main Apr 3, 2025
12 of 14 checks passed
@jiqing-feng jiqing-feng mentioned this pull request Apr 28, 2025
Merged
Guy-Bilitski pushed a commit to Guy-Bilitski/peft that referenced this pull request May 13, 2025
@jiqing-feng
TST Skip some GPU tests on non-CUDA devices (huggingface#2473)
5adab6b
efraimdahl pushed a commit to efraimdahl/peft that referenced this pull request Jul 12, 2025
@jiqing-feng @efraimdahl
TST Skip some GPU tests on non-CUDA devices (huggingface#2473)
ff0e09e
@jiqing-feng jiqing-feng deleted the test_2 branch October 9, 2025 01:42
@YangKai0616 YangKai0616 mentioned this pull request Oct 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BenjaminBossan BenjaminBossan BenjaminBossan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jiqing-feng @BenjaminBossan @HuggingFaceDocBuilderDev