Skip to content

fix: handle absent sys.modules entry in modeling_utils#44978

Closed
cjkindel wants to merge 2 commits intohuggingface:mainfrom
cjkindel:fix-sys-modules-keyerror
Closed

fix: handle absent sys.modules entry in modeling_utils#44978
cjkindel wants to merge 2 commits intohuggingface:mainfrom
cjkindel:fix-sys-modules-keyerror

Conversation

@cjkindel
Copy link
Copy Markdown

@cjkindel cjkindel commented Mar 24, 2026

What does this PR do?

_can_set_attn_implementation and _can_set_experts_implementation both do a direct subscript lookup into sys.modules:

class_module = sys.modules[cls.__module__]

If the module is not registered under its dotted name in sys.modules — which can happen with lazy loaders, frozen importers, or certain packaging environments on Windows — this raises a KeyError instead of returning False as intended.

The existing comment already acknowledges this class of problem ("This can happen for a custom model in a jupyter notebook or repl for example") but the file guard only ran after the subscript, so it never had a chance to fire.

First introduced: commit 0642963 (PR #42697, merged January 5, 2026).

Real-world traceback reported by Griptape Nodes users:

KeyError: 'transformers.models.clip.modeling_clip'
  modeling_utils.py, in _can_set_experts_implementation
    class_module = sys.modules[cls.__module__]

Reproducing code:

import sys
from transformers.models.clip.modeling_clip import CLIPTextModel, CLIPTextConfig

# Simulate condition where module is absent from sys.modules
del sys.modules["transformers.models.clip.modeling_clip"]

config = CLIPTextConfig()
CLIPTextModel(config)  # KeyError before this fix, succeeds after

Code Agent Policy

  • I confirm that this is not a pure code agent PR.

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?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@ArthurZucker (original author)
@Cyrilvallez (model loading)

Closes #45003

@cjkindel cjkindel marked this pull request as ready for review March 24, 2026 21:11
@cjkindel
Copy link
Copy Markdown
Author

cjkindel commented Mar 25, 2026

This isn't slop @Cyrilvallez , this is a legitimate problem affecting Griptape Nodes users with the Advanced Media library. Telling our customers to use my personal fork of Transformers isn't a healthy solution for us, even though it does mitigate the problem.

I acknowledge that Griptape Nodes does some dumb/hacky stuff with sys.modules but this does fix the problem and is 'right'. Without this change, our customers are facing errors when trying to use Flux.1 ControlNets with Diffusers.

@ArthurZucker
Copy link
Copy Markdown
Collaborator

Actually I think vllm had similar issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

modeling_utils unsafely accesses sys.modules[]

3 participants