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

Support Ascend NPU adapter loading #772

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Support Ascend NPU adapter loading #772

merged 1 commit into from
Aug 2, 2023

Conversation

ji-huazhong
Copy link
Contributor

@ji-huazhong ji-huazhong commented Aug 2, 2023

What does this PR do?

  • This PR introduces Lora and related adapter methods into npu since Accelerate has already provided support for npu.
  • Btw thanks to abhilash1910 for his outstanding contribution, it is more convenient to support third-party accelerators.

@ji-huazhong
Copy link
Contributor Author

ji-huazhong commented Aug 2, 2023

@BenjaminBossan @younesbelkada could you please review this PR? thanks🤗

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 2, 2023

The documentation is not available anymore as the PR was closed or merged.

Copy link
Contributor

@younesbelkada younesbelkada 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 adding this!

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.

LGTM, thx.

Just a more general question: Do we have any kind of test to show that NPU (or XPU for that matter) actually work?

@ji-huazhong
Copy link
Contributor Author

ji-huazhong commented Aug 2, 2023

LGTM, thx.

Just a more general question: Do we have any kind of test to show that NPU (or XPU for that matter) actually work?

@BenjaminBossan Thanks for your reply. I verified the effectiveness of this PR at LLaMA-Efficient-Tuning, which integrates peft.

If necessary, I could include additional test cases for third-party accelerators like NPU/XPU in a separate PR. This would be done under the assumption that these accelerators have cuda-like APIs within the PyTorch ecosystem.

@BenjaminBossan
Copy link
Member

I verified the effectiveness of this PR at LLaMA-Efficient-Tuning, which integrates peft.

Very nice, thanks for the pointer.

If necessary, I could include additional test cases for third-party accelerators like NPU/XPU in a separate PR. This would be done under the assumption that these accelerators have cuda-like APIs within the PyTorch ecosystem.

For me it was just important to ensure that someone has actually tested it. I don't know if we need these additional tests. On the one hand, it would be nice to have those to catch regressions, on the other hand they're probably not easy to integrate. I'll leave it up to the other maintainers to decide.

Copy link
Contributor

@pacman100 pacman100 left a comment

Choose a reason for hiding this comment

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

Thank you @statelesshz for adding NPU support, LGTM! 🚀

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.

5 participants