mirror of
https://github.com/modelscope/modelscope.git
synced 2026-08-29 10:08:40 +02:00
fix: resolve cross-repo auto_map when cache paths contain --
modelscope_hub 0.1.x layout embeds -- in local dirs; rejoining that
path into class_reference broke transformers' split("--"). Pass the
local snapshot as pretrained_model_name_or_path with a bare class name
instead, and stop mutating the args tuple in place.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -161,16 +161,29 @@ def _get_class_from_dynamic_module(class_reference, *args, **kwargs):
|
||||
When a config's ``auto_map`` references another repo, transformers calls
|
||||
``get_class_from_dynamic_module`` to fetch it. This wrapper ensures that
|
||||
fetch goes through ModelScope instead of HuggingFace.
|
||||
|
||||
Cross-repo ``auto_map`` entries use ``repo_id--module.Class``. After
|
||||
``snapshot_download``, the local cache path may itself contain ``--``
|
||||
(modelscope_hub 0.1.x layout: ``models/{owner}--{name}/snapshots/...``).
|
||||
Re-joining that path with ``--`` would make transformers'
|
||||
``class_reference.split("--")`` raise ``ValueError``. Instead, pass the
|
||||
local directory as ``pretrained_model_name_or_path`` and the bare
|
||||
``module.Class`` as ``class_reference`` so transformers takes the
|
||||
``os.path.isdir`` branch.
|
||||
"""
|
||||
from transformers.dynamic_module_utils import origin_get_class_from_dynamic_module
|
||||
if 'pretrained_model_name_or_path' in inspect.signature(
|
||||
origin_get_class_from_dynamic_module).parameters:
|
||||
has_pretrained_arg = (
|
||||
'pretrained_model_name_or_path'
|
||||
in inspect.signature(origin_get_class_from_dynamic_module).parameters)
|
||||
# ``args`` is a tuple; never mutate it in place.
|
||||
if has_pretrained_arg and args:
|
||||
pretrained_model_name_or_path = args[0]
|
||||
if not os.path.exists(pretrained_model_name_or_path):
|
||||
from modelscope import snapshot_download
|
||||
args[0] = snapshot_download(pretrained_model_name_or_path)
|
||||
args = (snapshot_download(pretrained_model_name_or_path), ) + args[1:]
|
||||
if '--' in class_reference:
|
||||
repo_id, class_reference = class_reference.split('--')
|
||||
# Only the first ``--`` is the auto_map delimiter (repo vs module).
|
||||
repo_id, class_reference = class_reference.split('--', 1)
|
||||
if not os.path.exists(repo_id):
|
||||
download_kwargs = {}
|
||||
extra_allow_file_pattern = _decide_allow_file_pattern(
|
||||
@@ -182,7 +195,14 @@ def _get_class_from_dynamic_module(class_reference, *args, **kwargs):
|
||||
download_kwargs['ignore_file_pattern'] = ignore_file_pattern
|
||||
from modelscope import snapshot_download
|
||||
repo_id = snapshot_download(repo_id, **download_kwargs)
|
||||
class_reference = repo_id + '--' + class_reference
|
||||
if has_pretrained_arg:
|
||||
# Local path + bare class name; do not rejoin with ``--``.
|
||||
args = (repo_id, ) + args[1:]
|
||||
else:
|
||||
# Legacy transformers without pretrained_model_name_or_path.
|
||||
# Unsafe if repo_id (local cache) contains '--'; modern
|
||||
# transformers always take the branch above.
|
||||
class_reference = repo_id + '--' + class_reference
|
||||
return origin_get_class_from_dynamic_module(class_reference, *args,
|
||||
**kwargs)
|
||||
|
||||
|
||||
@@ -249,19 +249,92 @@ class HFUtilTest(unittest.TestCase):
|
||||
f'Expected no weight files in {model_dir}, but found: '
|
||||
f"{[f for f in files if f.endswith('.safetensors') or f.endswith('.bin')]}"
|
||||
)
|
||||
cache_dir = os.path.dirname(model_dir)
|
||||
cache_dir = os.path.dirname(cache_dir)
|
||||
model_dir_2 = os.path.join(cache_dir, 'nomic-ai', 'nomic-bert-2048')
|
||||
if os.path.exists(model_dir_2):
|
||||
files = os.listdir(model_dir_2)
|
||||
has_weight_files = any(
|
||||
f.endswith('.safetensors') or f.endswith('.bin')
|
||||
for f in files)
|
||||
self.assertFalse(
|
||||
has_weight_files,
|
||||
f'Expected no weight files in {model_dir}, but found: '
|
||||
f"{[f for f in files if f.endswith('.safetensors') or f.endswith('.bin')]}"
|
||||
)
|
||||
# modelscope_hub 0.1.x layout uses models/{owner}--{name}/...;
|
||||
# older layout used {owner}/{name}/. Accept either.
|
||||
cache_root = model_dir
|
||||
for _ in range(4):
|
||||
parent = os.path.dirname(cache_root)
|
||||
if parent == cache_root:
|
||||
break
|
||||
cache_root = parent
|
||||
candidates = [
|
||||
os.path.join(cache_root, 'nomic-ai', 'nomic-bert-2048'),
|
||||
os.path.join(cache_root, 'models', 'nomic-ai--nomic-bert-2048'),
|
||||
]
|
||||
for model_dir_2 in candidates:
|
||||
if not os.path.exists(model_dir_2):
|
||||
continue
|
||||
# Walk into snapshots/{rev} if present.
|
||||
check_dirs = [model_dir_2]
|
||||
snapshots = os.path.join(model_dir_2, 'snapshots')
|
||||
if os.path.isdir(snapshots):
|
||||
check_dirs.extend(
|
||||
os.path.join(snapshots, d)
|
||||
for d in os.listdir(snapshots)
|
||||
if os.path.isdir(os.path.join(snapshots, d)))
|
||||
for check_dir in check_dirs:
|
||||
files = os.listdir(check_dir)
|
||||
has_weight_files = any(
|
||||
f.endswith('.safetensors') or f.endswith('.bin')
|
||||
for f in files)
|
||||
self.assertFalse(
|
||||
has_weight_files,
|
||||
f'Expected no weight files in {check_dir}, but found: '
|
||||
f"{[f for f in files if f.endswith('.safetensors') or f.endswith('.bin')]}"
|
||||
)
|
||||
|
||||
def test_dynamic_module_double_dash_cache_path(self):
|
||||
"""Cross-repo auto_map must survive cache paths that contain '--'.
|
||||
|
||||
modelscope_hub 0.1.x stores repos under ``models/{owner}--{name}/``.
|
||||
Rejoining that path into ``class_reference`` with ``--`` makes
|
||||
transformers' ``split("--")`` raise ValueError.
|
||||
"""
|
||||
from unittest import mock
|
||||
|
||||
from modelscope.utils.hf_util.patcher import \
|
||||
_get_class_from_dynamic_module
|
||||
|
||||
tmp = tempfile.mkdtemp()
|
||||
self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
|
||||
local_path = os.path.join(tmp, 'models', 'nomic-ai--nomic-bert-2048',
|
||||
'snapshots', 'rev')
|
||||
os.makedirs(local_path)
|
||||
pretrained = os.path.join(tmp, 'models',
|
||||
'nomic-ai--nomic-embed-text-v1.5',
|
||||
'snapshots', 'rev')
|
||||
os.makedirs(pretrained)
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_origin(class_reference,
|
||||
pretrained_model_name_or_path,
|
||||
*args,
|
||||
**kwargs):
|
||||
# Signature must match transformers so has_pretrained_arg is True.
|
||||
captured['class_reference'] = class_reference
|
||||
captured['pretrained'] = pretrained_model_name_or_path
|
||||
return type('DummyConfig', (), {})
|
||||
|
||||
class_ref = (
|
||||
'nomic-ai/nomic-bert-2048--'
|
||||
'configuration_hf_nomic_bert.NomicBertConfig')
|
||||
|
||||
# create=True: do not permanently leave origin_* on the module
|
||||
# (would break test_import_not_pollute_dynamic_module).
|
||||
with mock.patch(
|
||||
'transformers.dynamic_module_utils.origin_get_class_from_dynamic_module',
|
||||
new=fake_origin,
|
||||
create=True):
|
||||
with mock.patch(
|
||||
'modelscope.snapshot_download', return_value=local_path):
|
||||
_get_class_from_dynamic_module(class_ref, pretrained)
|
||||
|
||||
# Must pass bare module.Class (no '--') so transformers does not split.
|
||||
self.assertEqual(captured['class_reference'],
|
||||
'configuration_hf_nomic_bert.NomicBertConfig')
|
||||
# Local cache path (which contains '--') is pretrained_model_name_or_path.
|
||||
self.assertEqual(captured['pretrained'], local_path)
|
||||
|
||||
def test_import_not_pollute_dynamic_module(self):
|
||||
"""Importing from modelscope must not globally patch
|
||||
|
||||
Reference in New Issue
Block a user