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

Use a fork of gguf-tools to make it work on Windows #1704

Closed
wants to merge 1 commit into from

Conversation

zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Dec 14, 2024

Refs #1513.

https://github.com/antirez/gguf-tools is not being maintained, valid pull requests are left there months without getting a reply. Making gguf-tools work on Windows is not hard but with current state I can't get my work merged to upstream, so I think using a forked repo should be reasonable for now, and I can always try to merge things back once the upstream is alive.

My changes to gguf-tools are in https://github.com/frost-beta/gguf-tools/commits/main/, with a bonus side effect to fix antirez/gguf-tools#18, a MLX issue I was surprised to find out.

@awni
Copy link
Member

awni commented Dec 15, 2024

Thanks.. I'm not sure how critical it is to keep GGUF support in MLX at this point.

It's not used much, doesn't fit well with the rest of the serialization API since it's a higher-level model format, and if the package we rely on for it is not maintained it might be better to deprecate it or make it more of a third party tool that can load MLX models from GGUF directly.

CC @angeloskath @barronalex what do you all think about deprecating GGUF support / moving it out of MLX core?

@angeloskath
Copy link
Member

I think it is a good idea. It is probably hard to gauge how much it is used but it is definitely not well supported which is a good reason to deprecate and then remove from core.

@awni
Copy link
Member

awni commented Dec 20, 2024

@zcbenz I appreciate the effort on this but I don't think we should merge it right now though. I'm leaning towards deprecating GGUF support in MLX core and moving the GGUF support as a "third party extension" in our GGUF LLM example. My recommendation is to disable GGUF support for windows until we decide on that. If you are unhappy with the C++ test failing we can disable them when GGUF support is inactive.

@zcbenz
Copy link
Contributor Author

zcbenz commented Dec 21, 2024

We can disable the tests on Windows if some day you decided to run CI for it, I'm good just closing this PR.

@zcbenz zcbenz closed this Dec 21, 2024
@zcbenz zcbenz deleted the msvc-gguf branch December 21, 2024 01:02
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.

Please add an API for readonly files
3 participants