Skip to content

Conversation

@xiaoyu-work
Copy link
Collaborator

@xiaoyu-work xiaoyu-work commented Jan 16, 2026

This pull request introduces a major refactor of the HuggingFace ONNX I/O configuration system in Olive. The changes move away from hardcoded Python class registries and instead adopt a YAML-driven approach for specifying input/output configurations for both tasks and diffusers pipelines. This enables easier extensibility and clearer, data-driven configuration management. Additionally, the update adds default dummy input shapes and rich configuration templates for tasks and diffusers components.

The most important changes are:

YAML-based IO Configuration System:

  • Added new YAML files (defaults.yaml, tasks.yaml, diffusers.yaml) in olive/assets/io_configs/ that define default dummy input shapes, task templates, and diffusers component/pipeline specifications for ONNX export. These files replace the previous Python-based registries and provide a more flexible and maintainable configuration system. [1] [2] [3]

Refactor of IO Config Python Interface:

  • Replaced the contents of olive/common/hf/io_config/__init__.py to remove all hardcoded class registries and mapping logic, and instead import new YAML-driven utility functions (generate_dummy_inputs, get_io_config, get_diffusers_io_config, etc.). This greatly simplifies the interface and shifts responsibility to the new YAML configuration system.

Legal and Organizational Improvements:

  • Added copyright and license headers to olive/assets/__init__.py and olive/assets/io_configs/__init__.py.## Describe your changes

Checklist before requesting a review

  • Add unit tests for this change.
  • Make sure all tests can pass.
  • Update documents if necessary.
  • Lint and apply fixes to your code by running lintrunner -a
  • Is this a user-facing change? If yes, give a description of this change to be included in the release notes.

(Optional) Issue link

@xiaoyu-work xiaoyu-work changed the title Xiaoyu/io Refactor IO config to assets Jan 16, 2026
@xiaoyu-work xiaoyu-work changed the title Refactor IO config to assets Refactor IO config to yaml in assets Jan 16, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request introduces a major refactor of the HuggingFace ONNX I/O configuration system, migrating from hardcoded Python class registries to a flexible YAML-driven approach. The changes enable easier extensibility and clearer configuration management for model optimization workflows.

Changes:

  • Replaced Python-based OnnxConfig class registries with YAML configuration files for tasks and diffusers components
  • Introduced unified dummy input generators that read specifications from YAML rather than class hierarchies
  • Consolidated I/O configuration generation into a single task-driven API with automatic optional input filtering based on model signatures

Reviewed changes

Copilot reviewed 29 out of 34 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
olive/assets/io_configs/tasks.yaml Defines input/output specifications for 15+ HuggingFace tasks
olive/assets/io_configs/diffusers.yaml Defines I/O specs for diffusers components (SD, SDXL, SD3, Flux, Sana)
olive/assets/io_configs/defaults.yaml Provides default dimension values and attribute aliases for config resolution
olive/common/hf/io_config/yaml_loader.py YAML loading and config resolution utilities with LRU caching
olive/common/hf/io_config/task_config.py New unified API for generating I/O configs and dummy inputs
olive/common/hf/io_config/input_generators.py Refactored to use YAML-driven input generation
olive/common/hf/model_io.py Updated to use new task-driven API
olive/common/hf/wrapper.py Migrated to use resolve_alias for config attribute access
test/common/hf/io_config/*.py Updated tests for new YAML-driven system
Comments suppressed due to low confidence (1)

olive/common/hf/io_config/task_config.py:1

  • Missing check for whether 'with_past' key exists in task_spec before accessing it. This will raise a KeyError if a task doesn't define with_past overrides. Add a check: if self.use_past_in_inputs and 'with_past' in self.task_spec:
# -------------------------------------------------------------------------

xiaoyu-work and others added 3 commits January 20, 2026 12:38
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Comment on lines +198 to +199
height: [sample_size, image_size, vision_config.image_size]
width: [sample_size, image_size, vision_config.image_size]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous entries include the key in the value i.e. name is included in the list of aliases. These two entries don't. Should they? Or should the above 4 be updated to exclude the key name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! No need to put itself in the list. I'll update this.

# Add aliases if the same concept has different names
aliases:
my_custom_dim: [custom_dim, my_dim, special_dim]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above!

Comment on lines +14 to +15
height: [sample_size, image_size, vision_config.image_size]
width: [sample_size, image_size, vision_config.image_size]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem here too. height and weight are not included in the list of values.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this file something more specific - yaml_loader is too generic and says nothing about the actual functionality it provides.

YAML implementation is anyways private to the file. Public API seems more like utility functions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to io_resolver.


def _load_yaml(filename: str) -> dict[str, Any]:
"""Load a YAML file from the config directory."""
config_files = files("olive.assets.io_configs")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't importlib require user to create a whl and install the package for the system to pickup the change to config files.
(Unless of course if the user has the cloned repo in editable mode.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we move assets folder into olive package, so this change is for developers. If user has its own io config, they can specify it in the config file.

@shaahji
Copy link
Collaborator

shaahji commented Jan 21, 2026

Suggestion: Current implementation requires user to clone the git repo and modify it. This approach has challenges, most importantly dev vs. released version. Instead, rather than piling all the configuration in a single file, have one file hold configuration for one task. This will allow you to accept new task configuration on command line without explicitly modifying cloned repo. This approach also allows working with released version - which is always recommended compared to using a git clone.

P.S. That's the approach both lm-eval and bb implements.

@xiaoyu-work
Copy link
Collaborator Author

Suggestion: Current implementation requires user to clone the git repo and modify it. This approach has challenges, most importantly dev vs. released version. Instead, rather than piling all the configuration in a single file, have one file hold configuration for one task. This will allow you to accept new task configuration on command line without explicitly modifying cloned repo. This approach also allows working with released version - which is always recommended compared to using a git clone.

P.S. That's the approach both lm-eval and bb implements.

We now have 22 tasks, having separate config files for each task makes maintenance difficult. Most use cases would be adding new task IO rather than modifying existing ones. How about keeping the current configuration files, but also allowing override with external config files?

@shaahji
Copy link
Collaborator

shaahji commented Jan 21, 2026

Suggestion: Current implementation requires user to clone the git repo and modify it. This approach has challenges, most importantly dev vs. released version. Instead, rather than piling all the configuration in a single file, have one file hold configuration for one task. This will allow you to accept new task configuration on command line without explicitly modifying cloned repo. This approach also allows working with released version - which is always recommended compared to using a git clone.
P.S. That's the approach both lm-eval and bb implements.

We now have 22 tasks, having separate config files for each task makes maintenance difficult. Most use cases would be adding new task IO rather than modifying existing ones. How about keeping the current configuration files, but also allowing override with external config files?

How often do we expect to change any of these ones they are committed? For all intents and purposes, this is readonly data, unlikely to change ever. But if it makes it simpler, all for it.

On the implementation front, you could basically assume any number of tasks per file. So, there shouldn't be any difference whether you are loading the Olive released or the user provided one. Basically, the code path should remain the same.

@xiaoyu-work
Copy link
Collaborator Author

Suggestion: Current implementation requires user to clone the git repo and modify it. This approach has challenges, most importantly dev vs. released version. Instead, rather than piling all the configuration in a single file, have one file hold configuration for one task. This will allow you to accept new task configuration on command line without explicitly modifying cloned repo. This approach also allows working with released version - which is always recommended compared to using a git clone.
P.S. That's the approach both lm-eval and bb implements.

We now have 22 tasks, having separate config files for each task makes maintenance difficult. Most use cases would be adding new task IO rather than modifying existing ones. How about keeping the current configuration files, but also allowing override with external config files?

How often do we expect to change any of these ones they are committed? For all intents and purposes, this is readonly data, unlikely to change ever. But if it makes it simpler, all for it.

On the implementation front, you could basically assume any number of tasks per file. So, there shouldn't be any difference whether you are loading the Olive released or the user provided one. Basically, the code path should remain the same.

i think in real life, it is rare that user wants to provide his own io config for his own task. if a task is not supported by Olive, then we should add support it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants