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

[ModernBERT] Never save 'reference_compile' config; should be set based on end user #36305

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tomaarsen
Copy link
Member

What does this PR do?

  • Never save reference_compile in ModernBertConfig

Details

The ModernBERT architecture uses a reference_compile config option to determine whether various model layers are compiled. However, not every system supports this, so we either 1) use the user's provided value, 2) use the option in the config, or 3) set it automatically based on the user's system.

However, it seems that if you save a model, it will save reference_compile into the config. This is very problematic, as end users who cannot run compilation will be "forced" into compilation (option 2) instead of determining whether compilation is possible (option 3). This is quite annoying for smooth usage.

When this is merged and released, I'll consider writing a script that makes PRs on all ~850 ModernBERT models to remove the reference_compile option from their configs, so it's always just based on the user's system (or manual parameters).

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@ArthurZucker

  • Tom Aarsen

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.

1 participant