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

Improve gguf tensor processing #34515

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

VladOS95-cyber
Copy link
Contributor

What does this PR do?

Adding more and more architectures with custom weights conversion is making tensor processing system bigger, less flexible and readable.
Improve GGUF tensor processing system in modeling_gguf_pytorch_utils.py by adding TensorProcessor class for general cases without custom weights changes and all necessary separated tensor processor classes based on architecture with custom process logic.

Before submitting

Who can review?

Regarding the task @SunMarc @LysandreJik @ArthurZucker .

@VladOS95-cyber
Copy link
Contributor Author

Hey @SunMarc! I made some refactoring changes regarding tensor GGUF processing. Please, take a look. What do you think?

@VladOS95-cyber VladOS95-cyber force-pushed the improve-gguf-loading-system branch 3 times, most recently from 4b412ab to a090a84 Compare November 2, 2024 09:32
@VladOS95-cyber
Copy link
Contributor Author

Hey @SunMarc! Just a kind ping, does it make sense to have these changes?

Copy link
Member

@SunMarc SunMarc 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 making tensor processing cleaner. I think it makes sense to do that as it will be easier to read the code. Left a few minor comments. Can you have a second look @Isotr0py ?

src/transformers/modeling_gguf_pytorch_utils.py Outdated Show resolved Hide resolved
src/transformers/modeling_gguf_pytorch_utils.py Outdated Show resolved Hide resolved
@VladOS95-cyber
Copy link
Contributor Author

Hey @SunMarc , @Isotr0py! I made some refactoring based on your comments, please, take a look again.

Copy link
Contributor

@Isotr0py Isotr0py left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Thanks for improving this!

@VladOS95-cyber
Copy link
Contributor Author

Hey @SunMarc! Do you have any further comments or it looks good to you?

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.

3 participants