Skip to content

Fix: Add version check for timm to support mobilenetv5 models (fixes #39208) #39264

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

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

Conversation

VIGNESH15103
Copy link

What does this PR do?

Fixes issue #39208: Adds a version check for timm to ensure support for models like mobilenetv5_300m_enc, which require timm >= 0.9.10.

Motivation

The issue occurred because the mobilenetv5_300m_enc model name was not recognized in older versions of timm. This PR adds a conditional check to raise an informative ImportError if an unsupported version is detected.

Changes

  • Added a version check using packaging.version inside TimmWrapperImageProcessor.__init__.
  • Raises an error if a mobilenetv5 model is used with timm < 0.9.10.

Issue Link

Closes #39208

Reviewer Suggestion

@amyeroberts @qubvel (vision models)

Copy link
Member

@qubvel qubvel left a comment

Choose a reason for hiding this comment

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

Hey @VIGNESH15103, thanks a lot for the fix, I would suggest we add a check for config loading, not for the image processor

Comment on lines +63 to +70
# mobilenetv5 models require timm >= 0.9.10
if architecture is not None and architecture.startswith("mobilenetv5"):
if version.parse(timm.__version__) < version.parse("0.9.10"):
raise ImportError(
f"`mobilenetv5` models require `timm >= 0.9.10`, but found {timm.__version__}. "
"Please upgrade `timm` to the latest version."
)

Copy link
Member

Choose a reason for hiding this comment

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

The version for mibilenetv5 should be timm >= 1.0.16

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @qubvel! Just to confirm — would it be appropriate to add the version check inside TimmWrapperModel.init and TimmWrapperForImageClassification.init right before timm.create_model(...) based on config.architecture? I’ll update accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

Sound's good, but let's make it a bit more general, let's wrap timm.create_model to catch the RuntimeError and suggest to update timm with clear message

Copy link
Member

Choose a reason for hiding this comment

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

smt like

You are trying to instantiate `{architecture}`, but it's not supported in timm={timm.__version__}. Please, try updating to the latest timm version with `pip install -U timm`.

@VIGNESH15103 VIGNESH15103 force-pushed the fix-unknown-mobilenetv5 branch from 50ae8c5 to 22a2cee Compare July 8, 2025 21:02
@VIGNESH15103
Copy link
Author

Following the feedback:

  • I removed the version check from image_processing_timm_wrapper.py.
  • Instead, I now catch RuntimeError during timm.create_model(...) inside both TimmWrapperModel and TimmWrapperForImageClassification, as suggested.
  • Clear ImportError is raised when the architecture is unsupported, prompting users to upgrade timm.
  • Applied ruff formatting to resolve CI failures.

Kindly requesting a review and workflow approval when convenient. Thanks a lot!

Copy link
Contributor

github-actions bot commented Jul 9, 2025

[For maintainers] Suggested jobs to run (before merge)

run-slow: timm_wrapper

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.

Unknown Model (mobilenetv5_300m_enc) when loading Gemma 3n
2 participants