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

GeneratorArgs.is_torchtune_model is a misnomer #1273

Open
Jack-Khuu opened this issue Oct 5, 2024 · 0 comments
Open

GeneratorArgs.is_torchtune_model is a misnomer #1273

Jack-Khuu opened this issue Oct 5, 2024 · 0 comments
Assignees
Labels
actionable Items in the backlog waiting for an appropriate impl/fix good first issue Good for newcomers

Comments

@Jack-Khuu
Copy link
Contributor

🚀 The feature, motivation and pitch

is_torchtune_model is a misnomer and can result in buggy code. It gates logic for models that have tune suffix, but not all torchtune models end with this suffix. For example Flamingo (Llama3.2 11B) is also a torchtune model

This results in code like this:

if (
self.is_torchtune_model
or self.model.config.model_type == ModelType.Flamingo
):

Alternatives

No response

Additional context

No response

RFC (Optional)

While the logic around torchtune models itself needs some refactor, the short term solution is some combination of:

  • Rename the field to be more accurate
  • Refactor the logic such that it does account for torchtune models that do not end in tune
@Jack-Khuu Jack-Khuu added actionable Items in the backlog waiting for an appropriate impl/fix good first issue Good for newcomers labels Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable Items in the backlog waiting for an appropriate impl/fix good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants