Skip to content

Conversation

@kylesayrs
Copy link
Contributor

Purpose

  • This is a potential solution to the issue of conflicting Pixtral and Mistral configs, as detailed here
  • The downside to this approach is that (afaict) transformers makes an assumption that all configs have a one to one mapping with models. In this case, the PixtralTextConfig would map to the MistralModel, which is already associated with the MistralConfig. Additional logic and testing would be require to handle this case.

@Rocketknight1
Copy link
Member

cc @ArthurZucker - I think this is something that's usually handled with modular instead of directly importing the config, no?

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Was this not fixed by (#36077)?

@kylesayrs
Copy link
Contributor Author

@ArthurZucker This was fixed by #36077, however that solution is technically a hack as it saves more attributes than are necessary and expands the use of the is_composition beyond its original intended use.

I believe that this is a good solution for now, this PR can be reopened/forked if a better solution is desired.

@kylesayrs kylesayrs closed this Feb 26, 2025
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.

3 participants