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

Change umlaut test #11600

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Change umlaut test #11600

wants to merge 1 commit into from

Conversation

n00b001
Copy link

@n00b001 n00b001 commented Feb 2, 2025

Make sure to read the contributing guidelines before submitting a PR

@github-actions github-actions bot added the python python script changes label Feb 2, 2025
Copy link
Collaborator

@IMbackK IMbackK left a comment

Choose a reason for hiding this comment

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

Yeah that seams a bit immature.

Copy link
Collaborator

@JohannesGaessler JohannesGaessler left a comment

Choose a reason for hiding this comment

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

My understanding is that convert_hf_to_gguf_update.py is a dev script used to update the Python code in convert_hf_to_gguf.py via manual copy-pasting. If this PR were merged as-is the two files would I think become inconsistent. A review from someone who is actually knowledgeable about the Python conversion scripts would be appreciated.

Edit: I meant "actually knowledgeable" in reference to myself, not others.

@ggerganov
Copy link
Owner

The convert_hf_to_gguf_update.py script needs to be executed in order to update the vocab test input/outputs in models/ggml-vocab*.gguf.inp/.out. This change won't affect the contents of convert_hf_to_gguf.py because it is only modifying the unit tests and not the tokenizer list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python python script changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants