Skip to content

Add backwards-compatibility for older model format #526

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

Closed
wants to merge 1 commit into from
Closed

Add backwards-compatibility for older model format #526

wants to merge 1 commit into from

Conversation

thement
Copy link
Contributor

@thement thement commented Mar 26, 2023

Add support for reading older model files so that people do not have to throw out ggml alpaca models.

@thement thement marked this pull request as ready for review March 26, 2023 14:57
Copy link
Contributor

@anzz1 anzz1 left a comment

Choose a reason for hiding this comment

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

While I'm generally inclined to agree against forced deprecation, in this case allowing the use of the models generated with the old tokenizer without the embedded token score kinda defeats the purpose of having that safeguard in the first place.

As the change to the new tokenizer and versioning format was done pretty early in development, there shouldn't be too much old models floating around anymore. And regenerating the models with the available scripts is easy enough.

I can see how this could do more harm than good and possibly introduce hard-to-debug problems especially for new users.

Just simply accepting them like this would make it easier to

  1. use old models by accident, not understanding the consequences
  2. also have the negative effect of making them float around longer and kick the can forward of having to deal with this issue for longer

Hence, in my opinion, a better middle-ground solution would be creating a script which can convert the old models to the new format if someone has an absolute need for converting old models instead of generating new ones. It would be a conscious decision then to use the old format and understand that it might cause troubles, maybe have the script print a big disclaimer stating that "yes you can do this but you shouldn't, proceed if you understand what you're doing".

This is just my opinion though, would like to hear what the rest of the community has to say about this.

@marcom
Copy link
Contributor

marcom commented Mar 26, 2023

There has been a conversion script posted previously:

ggerganov/llama.cpp/issues/324#issuecomment-1476227818

with a link to this script:

https://gist.github.com/eiz/828bddec6162a023114ce19146cb2b82

I tried it once and it worked for me. Note that you might have to use the --n_parts 1 command-line arg when calling main with the converted params, as the script just generates a 1-part weights file.

@rabidcopy
Copy link
Contributor

I think the conversion script should be included in the repository somewhere to be honest. It's worked for every old format model I've thrown at it and saved me a lot of time.

@Green-Sky
Copy link
Collaborator

I think the conversion script should be included in the repository somewhere to be honest. It's worked for every old format model I've thrown at it and saved me a lot of time.

yea, i suggested that in that thread, and got approved. no one actually bothered to do it. @eiz

@thement thement closed this Mar 26, 2023
@thement
Copy link
Contributor Author

thement commented Mar 26, 2023

There has been a conversion script posted previously:

ggerganov/llama.cpp/issues/324#issuecomment-1476227818

with a link to this script:

https://gist.github.com/eiz/828bddec6162a023114ce19146cb2b82

I tried it once and it worked for me. Note that you might have to use the --n_parts 1 command-line arg when calling main with the converted params, as the script just generates a 1-part weights file.

OK, that was what I wanted to do in the first place but I could find the script. I'll make a merge request with this script instead.

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.

6 participants