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

Edits from finetuning #459

Merged
merged 12 commits into from
Jan 12, 2024
Merged

Edits from finetuning #459

merged 12 commits into from
Jan 12, 2024

Conversation

granawkins
Copy link
Member

Fix the fine-tuning output format, add a way for fine-tune models to exclude the system_prompt, and patch up some issues I found.

Pull Request Checklist

  • Documentation has been updated, or this change doesn't require that

@@ -170,6 +175,10 @@ class Model:
"text-embedding-ada-002": Model(
"text-embedding-ada-002", 8191, 0.0001, 0, embedding_model=True
),
# Fine-tuned on Jan-6 2024 with `sampler-one-hundred-v1.jsonl` data
"ft:gpt-3.5-turbo-1106:abante::8dsQMc4F": Model(
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately only abante ai org ai keys will be able to use this model so we shouldn't add it to the list.

Copy link
Member

@PCSwingle PCSwingle left a comment

Choose a reason for hiding this comment

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

I'm not sure this makes much sense; like @jakethekoenig mentioned, the finetuned models can only be used by us. How about we just makes the include_system_prompt a config option?

@granawkins
Copy link
Member Author

I get what you mean about the fine-tuned 3.5, makes sense ya I'll remove that one for now.

I do think it's good to build-in support for it though. I was planning to do a lil video about the end-to-end process of making our fine-tuned 3.5, with the thought that any of our users could do the same thing with their data.

Maybe known_models should try to match a base model if it's not immediately recognized? Just like I've done with the encoding function.

@jakethekoenig
Copy link
Member

I do think it's good to build-in support for it though. I was planning to do a lil video about the end-to-end process of making our fine-tuned 3.5, with the thought that any of our users could do the same thing with their data.

I like this idea. Back when I was doing this I thought we could eventually integrate it such that you could run a /finetune command and it would run on your own git repo and make fine tuning data, start a fine tuning job and automatically use the model when done. I think that approach to making training data didn't make much sense but the idea of eventually making /finetune a command or more simply distributing .jsonl's and documenting how to use them does make sense given that fine tuning is actually relatively cheap relative to inference.

Maybe known_models should try to match a base model if it's not immediately recognized? Just like I've done with the encoding function.

I like this idea. Mentat should know the costs to use a fine tuned gpt-3.5.

@granawkins
Copy link
Member Author

Added a wrapper class around known_models to handle fine-tuned models.

Removed the new requires_system_prompt stuff - if using a fine-tuned model that doesn't need a system prompt, users can use existing --no-parser-prompt arg:

mentat -a --model ft:gpt-3.5-turbo-1106:abante::8dsQMc4F --no-parser-prompt

mentat/config.py Outdated
@@ -36,17 +36,19 @@ class Config:
# Model specific settings
model: str = attr.field(
default="gpt-4-1106-preview",
metadata={"auto_completions": list(known_models.keys())},
metadata={"auto_completions": list(known_models.asdict().keys())},
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to do asdict? Isn't it technically a dict since it extends Dict?

Copy link
Member Author

Choose a reason for hiding this comment

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

thb I spent an hour trying to figure out how to have an overridden keys method return a type that my linter was happy with. Tried typing.KeyView, tried returning self.model.keys() directly - it wouldn't take.

attrs also has an asdict method which we use here and there so it seemed ok.

EDIT: nevermind I figured it out.

Copy link
Member

@PCSwingle PCSwingle left a comment

Choose a reason for hiding this comment

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

LGTM

@granawkins granawkins merged commit 0214b41 into main Jan 12, 2024
16 checks passed
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.

None yet

3 participants