diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c4b2f44efe..3406a860cc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,6 +7,32 @@ You can contribute in many different ways: giving ideas, answering questions, re Many thanks in advance to every contributor. +## Where does my contribution belong? + +Optimum is split into a core package (this repository) and several accelerator-specific +subpackages that live in their own repositories. Most bugs and features tied to a particular +hardware backend or export format should be addressed in the corresponding subpackage: + +| Feature / backend | Repository | +| :--------------------------------------------------------- | :------------------------------------------------------------------ | +| ONNX export and ONNX Runtime inference | [huggingface/optimum-onnx](https://github.com/huggingface/optimum-onnx) | +| Intel OpenVINO, NNCF, IPEX, neural-compressor | [huggingface/optimum-intel](https://github.com/huggingface/optimum-intel) | +| AMD Instinct GPUs and Ryzen AI NPU | [huggingface/optimum-amd](https://github.com/huggingface/optimum-amd) | +| Intel Gaudi Accelerators (HPU) | [huggingface/optimum-habana](https://github.com/huggingface/optimum-habana) | +| AWS Trainium and Inferentia (Neuron) | [huggingface/optimum-neuron](https://github.com/huggingface/optimum-neuron) | +| NVIDIA TensorRT-LLM | [huggingface/optimum-nvidia](https://github.com/huggingface/optimum-nvidia) | +| PyTorch quantization (Quanto) | [huggingface/optimum-quanto](https://github.com/huggingface/optimum-quanto) | +| FuriosaAI | [huggingface/optimum-furiosa](https://github.com/huggingface/optimum-furiosa) | +| ExecuTorch (on-device inference) | [huggingface/optimum-executorch](https://github.com/huggingface/optimum-executorch) | + +This repository (`optimum`) holds the shared core: the `optimum-cli` entry point, the common +task/model mapping in `optimum/exporters/tasks.py`, shared utilities, and the pipelines/GPTQ/fx +modules. If a bug is reproducible without any accelerator extras installed (for example in +`TasksManager` library/task inference or in the CLI command resolution), it belongs here. + +If in doubt, open the issue in this repository and a maintainer will redirect it to the right +subpackage if needed. + ## How to work on an open Issue? You have the list of open Issues at: https://github.com/huggingface/optimum/issues diff --git a/optimum/commands/optimum_cli.py b/optimum/commands/optimum_cli.py index 74ee549f8e..1f65fcd1c3 100644 --- a/optimum/commands/optimum_cli.py +++ b/optimum/commands/optimum_cli.py @@ -121,14 +121,29 @@ def load_optimum_namespace_cli_commands() -> ( # Find all registration files and load the commands to register commands_to_register = [] - for register_path in set(commands_register_spec.submodule_search_locations): + # Deduplicate by the resolved physical path: on some systems (e.g. RHEL) `lib64` is a + # symlink to `lib`, so `submodule_search_locations` can return two entries pointing to the + # same physical directory. A plain set() over the raw strings does not deduplicate those, + # which leads to commands being registered twice and spurious __init__.py warnings (#2417). + seen_register_paths = set() + for register_path in commands_register_spec.submodule_search_locations: register_path = Path(register_path) if not register_path.is_dir(): # skip non-directory paths continue + resolved_register_path = register_path.resolve() + if resolved_register_path in seen_register_paths: + # already processed this physical directory (e.g. via a lib64 -> lib symlink) + continue + seen_register_paths.add(resolved_register_path) # Look for python files - for register_file in register_path.iterdir(): + try: + register_files = list(register_path.iterdir()) + except OSError as e: + logger.warning(f"Could not iterate over register directory {register_path}: {e}") + continue + for register_file in register_files: if register_file.name == "__init__.py": logger.warning( "The namespace optimum.commands.register should never contain an __init__.py file (PEP 420). " diff --git a/optimum/exporters/tasks.py b/optimum/exporters/tasks.py index 9e0470b1ca..1cb6173046 100644 --- a/optimum/exporters/tasks.py +++ b/optimum/exporters/tasks.py @@ -1181,7 +1181,14 @@ def get_model_from_task( ) if library_name == "timm": - model = model_class(f"hf_hub:{model_name_or_path}", pretrained=True, exportable=True) + # timm selects its loading source via a prefix: `hf_hub:` (aliased to `hf-hub:`) + # downloads from the Hugging Face Hub, while `local-dir:` loads from a local folder. + # A bare local path is rejected by timm, so we pick the prefix based on whether the + # argument is a local directory. See #2423. + if os.path.isdir(model_name_or_path): + model = model_class(f"local-dir:{model_name_or_path}", pretrained=True, exportable=True) + else: + model = model_class(f"hf_hub:{model_name_or_path}", pretrained=True, exportable=True) model = model.to(torch_dtype).to(device) elif library_name == "sentence_transformers": token = model_kwargs.pop("token", None) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 167aef30b1..82cd751a67 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -13,13 +13,17 @@ # See the License for the specific language governing permissions and # limitations under the License. +import importlib import inspect import os import shutil import subprocess +import sys import tempfile +import types import unittest from pathlib import Path +from unittest import mock import optimum.commands.base @@ -88,3 +92,68 @@ def test_register_command(self): def tearDown(self): super().tearDown() REGISTERED_CLI_WITH_CUSTOM_COMMAND_PATH.unlink(missing_ok=True) + + def test_load_namespace_cli_commands_dedup_symlink(self): + # Regression test for #2417: on systems where `lib64` is a symlink to `lib`, + # `submodule_search_locations` can return two entries pointing to the same physical + # directory. Commands from that directory must be registered only once. + from optimum.commands.optimum_cli import load_optimum_namespace_cli_commands + + register_module_name = "cli_2417_dedup_check" + register_file_content = ( + "from optimum.commands.base import BaseOptimumCLICommand, CommandInfo\n" + "\n" + "\n" + "class Cli2417DedupCheckCommand(BaseOptimumCLICommand):\n" + " COMMAND = CommandInfo(name='cli-2417-dedup-check', help='dedup test')\n" + "\n" + " def run(self):\n" + " pass\n" + "\n" + "\n" + "REGISTER_COMMANDS = [Cli2417DedupCheckCommand]\n" + ) + + with tempfile.TemporaryDirectory() as tmp: + real_register_dir = Path(tmp) / "register" + real_register_dir.mkdir() + (real_register_dir / f"{register_module_name}.py").write_text(register_file_content) + symlink_register_dir = Path(tmp) / "register_symlink" + symlink_register_dir.symlink_to(real_register_dir, target_is_directory=True) + + fake_spec = importlib.machinery.ModuleSpec("optimum.commands.register", loader=None, is_package=True) + fake_spec.submodule_search_locations = [str(real_register_dir), str(symlink_register_dir)] + # Provide a namespace parent whose __path__ points at our isolated directory so that + # `importlib.import_module("optimum.commands.register.cli_2417_dedup_check")` resolves there. + fake_parent = types.ModuleType("optimum.commands.register") + fake_parent.__path__ = [str(real_register_dir)] + fake_parent.__spec__ = fake_spec + + original_find_spec = importlib.util.find_spec + original_parent = sys.modules.get("optimum.commands.register", None) + original_submodule = sys.modules.get(f"optimum.commands.register.{register_module_name}", None) + + def patched_find_spec(name, *args, **kwargs): + if name == "optimum.commands.register": + return fake_spec + return original_find_spec(name, *args, **kwargs) + + try: + sys.modules["optimum.commands.register"] = fake_parent + with mock.patch("importlib.util.find_spec", side_effect=patched_find_spec): + commands = load_optimum_namespace_cli_commands() + finally: + if original_parent is None: + sys.modules.pop("optimum.commands.register", None) + else: + sys.modules["optimum.commands.register"] = original_parent + sys.modules.pop(f"optimum.commands.register.{register_module_name}", None) + if original_submodule is not None: + sys.modules[f"optimum.commands.register.{register_module_name}"] = original_submodule + + command_names = [command.COMMAND.name for command, _ in commands] + self.assertEqual( + command_names.count("cli-2417-dedup-check"), + 1, + f"Expected the command to be registered exactly once, but got: {command_names}", + ) diff --git a/tests/exporters/common/test_timm_loading.py b/tests/exporters/common/test_timm_loading.py new file mode 100644 index 0000000000..16062e2ac5 --- /dev/null +++ b/tests/exporters/common/test_timm_loading.py @@ -0,0 +1,59 @@ +# coding=utf-8 +# Copyright 2025 The HuggingFace Team. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import importlib +import tempfile +from unittest import TestCase, mock + +from optimum.exporters.tasks import TasksManager + + +def _load_timm_via_tasks_manager(model_name_or_path: str) -> mock.MagicMock: + """Runs the timm loading branch of `TasksManager.get_model_from_task` with a fake timm module + and returns the `create_model` mock, so the test can assert which source prefix was used.""" + fake_timm = mock.MagicMock() + create_model = mock.MagicMock() + fake_timm.create_model = create_model + + real_import_module = importlib.import_module + + def patched_import(name, *args, **kwargs): + if name == "timm": + return fake_timm + return real_import_module(name, *args, **kwargs) + + with mock.patch("optimum.exporters.tasks.importlib.import_module", side_effect=patched_import): + TasksManager.get_model_from_task( + task="image-classification", + model_name_or_path=model_name_or_path, + framework="pt", + library_name="timm", + ) + return create_model + + +class TimmLocalDirLoadingTestCase(TestCase): + def test_local_timm_path_uses_local_dir_prefix(self): + # Regression test for #2423: a local directory must be loaded with the `local-dir:` prefix, + # not `hf_hub:` (which timm would interpret as a Hub repo id and fail to download). + with tempfile.TemporaryDirectory() as local_dir: + create_model = _load_timm_via_tasks_manager(local_dir) + create_model.assert_called_once_with(f"local-dir:{local_dir}", pretrained=True, exportable=True) + + def test_hub_timm_path_uses_hf_hub_prefix(self): + # A non-local (Hub repo id) argument must keep using the `hf_hub:` prefix, as before. + create_model = _load_timm_via_tasks_manager("timm/mobilenetv3_large_100.ra_in1k") + create_model.assert_called_once_with( + "hf_hub:timm/mobilenetv3_large_100.ra_in1k", pretrained=True, exportable=True + )