Repository navigation
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
@ydshieh Please review, thank You |
|
Hi @rigen1048 , could you provide some context:
|
|
Hi @ydshieh, Thank you for the follow-up. Here is the context regarding the issue and the fix: 1. Error MessageThe core failure seen in CI/CD is: This occurs specifically during dynamic module loading (e.g., in 2. Root Cause & ReproducibilityThe issue is a synchronization problem between This is not easily reproducible locally by simply running the 3. Is it only reproducible on MPS?The answer is "no and yes":
4. The SolutionFollowing the excellent suggestion from @tarekziade in #45787, I have implemented the fix in import importlib
from transformers.dynamic_module_utils import init_hf_modules
init_hf_modules()
importlib.invalidate_caches()This approach directly resolves the race condition by ensuring the environment is correctly configured for all workers before tests begin. |
|
Thanks for the context. My memory might be wrong but I didn't see/remember this failure previously, that why I am wondering if this failure only occurs recently due to a new merge PR. If you happen to know which CircleCI job run of which PR that shows this failure, it would be great. Or at least which test file. I guess the most relevant one is |
|
For the fix: I talked to @tarekziade . Although I think the current PR is a simple and working solution, I am not in favor of this approach. Could we change from if not init_path.exists():
init_path.touch()
importlib.invalidate_caches()to if not init_path.exists():
init_path.touch()
importlib.invalidate_caches()Or this won't help? |
|
I have reverted the conftest.py modification and applied the suggested change to dynamic_module_utils.py, but unfortunately, it did not resolve the issue in my reproduction test. The only 2 independent approaches that successfully fixed the ModuleNotFoundError in my tests were:
I don't believe there is a bias in my test for a specific method, as it consistently reproduces the failure otherwise. I would appreciate it if you could take a look at the reproduction script." |
|
Could you share the reproduction script. I didn't find it by search in this PR. |
|
Oh Yes I thought it should not be part of the merge. |
|
You can paste in a comment, something like Your script code ... |
import importlib
import os
import shutil
import sys
from pathlib import Path
from transformers.dynamic_module_utils import get_class_in_module
from transformers.testing_utils import TestCasePlus
from transformers.utils import HF_MODULES_CACHE
class DynamicImportRegressionTest(TestCasePlus):
def setUp(self):
super().setUp()
self.module_name = "modeling_regression"
self.class_name = "RegressionConfig"
self.san_org = "test_regression_org"
self.repo = "test_repo"
self.hash = "v1"
# Manually populate the cache with a module that uses relative imports.
# This mirrors the state of a 'Worker B' in a parallel pytest-xdist run.
self.base_path = Path(HF_MODULES_CACHE) / "transformers_modules" / self.san_org / self.repo / self.hash
if self.base_path.exists():
shutil.rmtree(Path(HF_MODULES_CACHE) / "transformers_modules" / self.san_org)
os.makedirs(self.base_path, exist_ok=True)
# Create __init__.py files for the package structure
curr = Path(HF_MODULES_CACHE) / "transformers_modules"
for part in [self.san_org, self.repo, self.hash]:
curr = curr / part
os.makedirs(curr, exist_ok=True)
(curr / "__init__.py").touch()
# Create a utility file for relative import
with open(self.base_path / "utils.py", "w", encoding="utf-8") as f:
f.write("def get_val(): return 42\n")
# Create the main modeling file with a relative import
with open(self.base_path / f"{self.module_name}.py", "w", encoding="utf-8") as f:
f.write("from transformers import PretrainedConfig\n")
f.write("from .utils import get_val\n")
f.write(f"class {self.class_name}(PretrainedConfig):\n")
f.write(" model_type = 'regression'\n")
def tearDown(self):
super().tearDown()
if self.base_path.exists():
shutil.rmtree(Path(HF_MODULES_CACHE) / "transformers_modules" / self.san_org)
def test_get_class_in_module_with_relative_import(self):
"""
Verifies that get_class_in_module can successfully load a module with relative imports
in a fresh process environment. This test passes if the environment is correctly
initialized by either the library or the test configuration (conftest.py).
"""
# Clear existing modules to force a fresh import
for mod in list(sys.modules.keys()):
if mod.startswith(f"transformers_modules.{self.san_org}"):
del sys.modules[mod]
importlib.invalidate_caches()
# The path relative to HF_MODULES_CACHE
rel_module_path = f"transformers_modules/{self.san_org}/{self.repo}/{self.hash}/{self.module_name}.py"
# This call will fail with ModuleNotFoundError if the HF_MODULES_CACHE
# is not present in sys.path.
try:
cls = get_class_in_module(self.class_name, rel_module_path)
self.assertEqual(cls.__name__, self.class_name)
except ModuleNotFoundError as e:
self.fail(f"Dynamic import failed. The environment was not correctly initialized. Error: {e}") |
|
thanks a lot, i will check and see if I can get something more clear |
|
Sure love being helpful |
|
Hi @ydshieh, just checking if you had any progress on the cleaner approach? |
What does this PR do?
This PR fixes a distributed race condition in dynamic_module_utils.py that causes intermittent ModuleNotFoundError: No module named 'transformers_modules' failures when running tests with pytest-xdist, faced in #45687 pr & (maybe) #45818 pr ci/cd test
Root Cause
When running in parallel (e.g., -n 8), each worker process has an isolated sys.path.
fix:
As @tarekziade suggested in #45787 pr
Reason for Opening the same pr:
I mistakenly ruined the git history of the last pr, So.....
Code Agent Policy
The Transformers repo is currently being overwhelmed by a large number of PRs and issue comments written by
code agents. We are currently bottlenecked by our ability to review and respond to them. As a result,
we ask that new users do not submit pure code agent PRs at this time.
You may use code agents in drafting or to help you diagnose issues. We'd also ask autonomous "OpenClaw"-like agents
not to open any PRs or issues for the moment.
PRs that appear to be fully agent-written will probably be closed without review, and we may block users who do this
repeatedly or maliciously.
This is a rapidly-evolving situation that's causing significant shockwaves in the open-source community. As a result,
this policy is likely to be updated regularly in the near future. For more information, please read
CONTRIBUTING.md.Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
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.
@ydshieh