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

Instructions to raise PR for addition of shared library files(.so) and .cpp files #35492

Open
vineel96 opened this issue Jan 2, 2025 · 3 comments
Labels
Feature request Request for a new feature

Comments

@vineel96
Copy link

vineel96 commented Jan 2, 2025

Feature request

Hello @ArthurZucker @zucchini-nlp @Rocketknight1 @gante,
I have an proposal for improvement of Mamba model for which I wanted to raise PR.
Method: The code changes are made in c++ file. I use cython to integrate c++ code to python. For this I will be generating an shared library file (.so) which will have the code. I will import it as library in transformers python code(Ex: import library) and call API's (Ex: library.api_name() ) from it.

I have queries on how to raise a PR for the above scenario:

  1. In which folder path should I keep the cython c++ binding codes ?
  2. How to provide the share library file (.so) file in the PR, In which location should I keep?

Motivation

I will be detailing the motivation when I raise the PR for it. Now I need to instructions to raise the PR

Your contribution

I will be submitting a PR.

@vineel96 vineel96 added the Feature request Request for a new feature label Jan 2, 2025
@vineel96 vineel96 changed the title Instructions to raise PR for a particular changes Instructions to raise PR for addition of shared library files(.so) and .cpp files Jan 2, 2025
@vineel96
Copy link
Author

vineel96 commented Jan 4, 2025

@ArthurZucker @zucchini-nlp @Rocketknight1 @gante, Any help regarding this?

@zucchini-nlp
Copy link
Member

We usually have kernels for CUDA/CPU here https://github.com/huggingface/transformers/tree/main/src/transformers/kernels, so I believe mamba will also belong there

@vineel96
Copy link
Author

vineel96 commented Jan 7, 2025

@zucchini-nlp , To bind c++ code with python this repo is using torch.utils.cpp_extension to load ".cpp" extension files and create shared file(.so) (https://github.com/huggingface/transformers/blob/main/src/transformers/models/mra/modeling_mra.py#L59 ) . But torch.utils.cpp_extension uses pybindings to bind c++ code with python. In our experiments, we found that cython is faster than pybindings when we call c++ code from python. So can we integrate using cython? or it is mandatory to follow what transformers repo uses by default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

2 participants