-
Notifications
You must be signed in to change notification settings - Fork 28.1k
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
Add utility for Reload Transformers imports cache for development workflow #35508 #35858
Add utility for Reload Transformers imports cache for development workflow #35508 #35858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for my late review! Thanks for the PR 🤗 just missing a bit of doc, maybe in How to hack a model? As I guess this is the usecase? Also mentionning when would people use this (based of the issue!)
Hey @ArthurZucker , i have added the necessary documentation in 388b2ac . Do tell me if anything else is required , i'll make those fixes as well . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good to go, thanks
Hey! This test is failing in the CI |
anything specific i need to pass them @Rocketknight1 ? i'll do it right away . |
Hi @Rocketknight1, I've done a thorough analysis of the failing tests in relation to the changes introduced in this PR. Let me break down each failure type and explain why I believe they're unrelated to these changes:
These failures occur in question-answering pipeline tests across ERNIE, ALBERT, and RemBERT models. They're specifically about NumPy array dimensionality operations. The changes from the PR only touch the module import system and don't interact with any NumPy operations or array handling.
This is related to model weight deserialization. The
While this is import-related, the cache clearing function:
These are basic assertion failures in test logic, unrelated to module importing or caching mechanisms. The changes in this PR are focused solely on providing a development utility to clear the transformers module cache when needed. The function:
That said if you see any way that the changes from the PR could be causing these failures, I'm more than happy to investigate further and make any necessary corrections. Please let me know if you'd like me to explore any specific aspects of the implementation. |
…kflow huggingface#35508 (huggingface#35858) * Reload transformers fix form cache * add imports * add test fn for clearing import cache * ruff fix to core import logic * ruff fix to test file * fixup for imports * fixup for test * lru restore * test check * fix style changes * added documentation for usecase * fixing --------- Co-authored-by: sambhavnoobcoder <[email protected]>
@sambhavnoobcoder : the test seems to somehow invalidate the state of the test engine. Some (not all) of the next tests start to fail if executed after this one though otherwise they pass. See: |
thank you for pointing it out . #36345 PR solves this issue . Hoping that gets merged quickly and solves this issue . |
Problem Statement
When developing or modifying Transformers models, cached imports can prevent code changes from being picked up without manually restarting Python. This makes the development workflow more cumbersome, especially when making iterative changes to model implementations.
Fixes #35508
Root Cause Analysis
The issue stems from Python's module caching behavior in
sys.modules
. Additionally, Transformers uses a custom_LazyModule
system for imports that maintains its own object cache. Both these caches need to be cleared to fully reload modified code.Implementation
Added a
clear_import_cache()
utility function totransformers.utils.import_utils
that:sys.modules
_LazyModule
instancessys.modules
The implementation is minimally invasive and works with Transformers' existing module system.
Testing
Added a test case that verifies:
sys.modules
The test ensures the cache clearing is thorough while maintaining backward compatibility.
Screenshots
cc : @ArthurZucker @SunMarc @Rocketknight1 and anyone interested in development workflow improvements.