No more forcing separators#2279
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. |
There was a problem hiding this comment.
Pull Request Overview
This PR removes the forced conversion of model type names by no longer replacing underscores with dashes, instead using the official model names from transformers. Key changes include updating test cases, exporter configurations, and documentation comments across multiple files to reflect the new naming convention without separator modifications.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/onnxruntime/testing_utils.py | Updated model name keys to use official naming (e.g., "blenderbot-small"). |
| tests/onnxruntime/test_optimization.py and test_modeling.py | Removed use of the replace() function when checking supported tasks. |
| tests/exporters/utils.py, tflite, onnx, etc. | Modified model type keys to align with official naming conventions. |
| optimum/utils/normalized_config.py and related exporter files | Eliminated forced underscore-to-dash conversion in model type handling. |
| optimum/exporters/tasks.py | Updated task mappings and removed deprecated string manipulation. |
| Other files across tests and exporter modules | Consistently applied new naming rules in both code and comments. |
Comments suppressed due to low confidence (2)
optimum/utils/normalized_config.py:306
- With the removal of forced separator conversion, please update the documentation to clearly specify the expected model_type format and ensure that users are aware of the new naming convention.
model_type = model_type.replace("_", "-")
optimum/exporters/tasks.py:1447
- Since the transformation of model_type values has been removed, confirm that the documentation and internal guidelines are updated accordingly to reflect the new behavior.
model_type = model_type.lower().replace("_", "-")
echarlaix
left a comment
echarlaix
left a comment
There was a problem hiding this comment.
LGTM, thanks @IlyasMoutawwakil
| @@ -459,7 +459,7 @@ class TasksManager: | |||
| "question-answering", | |||
| onnx="BigBirdOnnxConfig", | |||
| ), | |||
There was a problem hiding this comment.
would you mind also checking that this doesn't break anything for others optimum-xxx like optimum-intel or optimum-neuron
There was a problem hiding this comment.
done for optimum-intel in huggingface/optimum-intel#1329
for optimum-neuron I see that they are still pinning optimum 1.23 🤔
I could give it a shot but only if they wanna also update @JingyaHuang
|
here's the script for reference, it should always be ran with latest transformers from transformers.models.auto.configuration_auto import CONFIG_MAPPING_NAMES
def replace_in_directory(directory, old_str, new_str):
import os
for root, dirs, files in os.walk(directory):
for file in files:
file_path = os.path.join(root, file)
# skip this file
if file == "script.py":
continue
# skip non .py .txt and .json files
if not (file.endswith(".py") or file.endswith(".txt") or file.endswith(".json")):
continue
try:
with open(file_path, "r", encoding="utf-8") as f:
content = f.read()
if old_str in content:
content = content.replace(old_str, new_str)
with open(file_path, "w", encoding="utf-8") as f:
f.write(content)
except (UnicodeDecodeError, FileNotFoundError):
# Skip files that cannot be read or do not exist
continue
def correct_model_types():
for model_type in CONFIG_MAPPING_NAMES.keys():
if "-" in model_type:
wrong_model_type = model_type.replace("-", "_")
elif "_" in model_type:
wrong_model_type = model_type.replace("_", "-")
else:
continue
new_str = f'"{model_type}"'
old_str = f'"{wrong_model_type}"'
# replace all occurrences in current directory
replace_in_directory(".", old_str, new_str)
def remove_separators_shenanigans():
old_str = '.replace("_", "-")'
new_str = ""
replace_in_directory(".", old_str, new_str)
old_str = '.replace("-", "_")'
new_str = ""
replace_in_directory(".", old_str, new_str)
if __name__ == "__main__":
correct_model_types()
remove_separators_shenanigans()
print("Model types corrected.") |
|
thanks a lot @IlyasMoutawwakil !! if that works for you we can merge after the next release to have some time to make sure this doesn't introduce any issue for other repos (optimum-neuron / optimum-executorch and others) |
|
cc @JingyaHuang let us know if that works for you (might have an impact on optimum-neuron) |
What does this PR do?
There's a lot of separators shenanigans we have to do every time we deal with model_types (or is it model-types 😅).
This PR removes that by using the names models were given in transformers.
Motivation for this PR is that in #2278 I was able to add the officially named "qwen3_moe" and hardest part was figuring out where it should be "qwen3-moe" and where "qwen3_moe" should be used.
Before submitting
Who can review?