Skip to content

mypy: Remove use of undefined GGUFWriter symbols#1287

Closed
booxter wants to merge 1 commit intoinstructlab:mainfrom
booxter:remove-undefined-n-experts-counts-gguf
Closed

mypy: Remove use of undefined GGUFWriter symbols#1287
booxter wants to merge 1 commit intoinstructlab:mainfrom
booxter:remove-undefined-n-experts-counts-gguf

Conversation

@booxter
Copy link
Contributor

@booxter booxter commented Jun 6, 2024

Neither add_expert_count nor add_expert_used_count are defined in the version of gguf library we use. Actually, these symbols are not yet released on pypi, and are available in git repo of the library only:

ggml-org/llama.cpp@799a1cb

This makes mypy complain about it.

This patch removes code related to these parameters. If and when we upgrade to gguf version that supports them, we may revert this patch.

Neither add_expert_count nor add_expert_used_count are defined in the
version of gguf library we use.  Actually, these symbols are not yet
released on pypi, and are available in git repo of the library only:

ggml-org/llama.cpp@799a1cb

This makes mypy complain about it.

This patch removes code related to these parameters. If and when we
upgrade to gguf version that supports them, we may revert this patch.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
@booxter
Copy link
Contributor Author

booxter commented Jun 6, 2024

Overall, this module seems to be a copy-pasted version of convert.py from gguf library repo. Shouldn't we reuse the code from the original library?.. @xukai92 you added this file here. What's the story behind it?

UPD: nevermind, I think I understand now: the file is not part of the gguf library, so it's impossible to get it through a package... I guess we'll maintain it ourselves then. But then the question is - do we want to enable mypy checks for the code that is not fully owned by us? What's the policy for this file?

@leseb leseb requested a review from xukai92 June 10, 2024 15:12
@booxter
Copy link
Contributor Author

booxter commented Jun 12, 2024

Squashed this into #1275

@booxter booxter closed this Jun 12, 2024
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.

1 participant