Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make optimized ONNX external data files downloadable from Hugging Face Hub #1725

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Make optimized ONNX external data files downloadable from Hugging Face Hub #1725

wants to merge 3 commits into from

Conversation

kazssym
Copy link

@kazssym kazssym commented Feb 27, 2024 •
edited

What does this PR do?

This PR adds .data to the external data file suffixes so that an optimized model can be downloaded from Hugging Face as the optimizer saves models with .data instead of _data.

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@kazssym
Rename downloaded Hugging Face model data file suffix
c59d942 
This commit renames the downloaded Hugging Face model data file from
"${file_name}_data" to "${file_name}.data".

This change aligns the file naming convention with other downloaded model data files in the project, improving consistency and clarity.
@kazssym kazssym changed the title Rename downloaded Hugging Face model data file suffix Rename downloadable Hugging Face model data file suffix Feb 27, 2024
@kazssym kazssym changed the title Rename downloadable Hugging Face model data file suffix Correct downloadable Hugging Face model data file suffix Feb 28, 2024
@fxmarty
Copy link
Collaborator

fxmarty commented Feb 28, 2024 •
edited

Hi @kazssym, Optimum ONNX export uses model.onnx_data for the external data file, contrary to other tools that use model.onnx.data. Hence the check on the Hub. Has this caused problems to you?

@kazssym
Copy link
Author

kazssym commented Feb 29, 2024 •
edited

Hi @kazssym, Optimum ONNX export uses model.onnx_data for the external data file, contrary to other tools that use model.onnx.data. Hence the check on the Hub. Has this caused problems to you?

When I stored my external data file as .onnx_data on the Hub, it could be downloaded by Optimum but ONNX Runtime failed to load the file as it assumes .onnx.data internally. Renaming to .onnx.data on the Hub makes Optimum ignore the file and ONNX Runtime cannot use it anyway. It is inconvenient if I must manually download model data from the Hub.

DELETED

@fxmarty
Copy link
Collaborator

fxmarty commented Feb 29, 2024

Thank you @kazssym, I am not sure to understand. Could you open an issue about that with a reproduction of the issue?

For example,

optimum-cli export onnx --model saibo/llama-1B llama_onnx

followed by

from optimum.onnxruntime import ORTModelForCausalLM

model = ORTModelForCausalLM.from_pretrained("/path/to/llama_onnx")

works for me.

@kazssym
Copy link
Author

kazssym commented Feb 29, 2024 •
edited

Thank you @kazssym, I am not sure to understand. Could you open an issue about that with a reproduction of the issue?

For example,

optimum-cli export onnx --model saibo/llama-1B llama_onnx

followed by

from optimum.onnxruntime import ORTModelForCausalLM



model = ORTModelForCausalLM.from_pretrained("/path/to/llama_onnx")

works for me.

I tried to upload an exported ONNX model to Hugging Face Hub and let it be downloaded with Optimum.

I must upload it with _data suffix but ONNX Runtime could not find it.

https://huggingface.co/kazssym/stablelm-3b-4e1t-onnx-fp32/tree/b9c9eecad782f82dbb163448019a7d2ea1c3f2f2

Then I renamed the file with .data and it could not be downloaded by Optimum.

@fxmarty
Copy link
Collaborator

fxmarty commented Feb 29, 2024

@kazssym Thanks, let me try.

@kazssym kazssym marked this pull request as draft February 29, 2024 11:39
@kazssym
Copy link
Author

kazssym commented Feb 29, 2024 •
edited

I found it needs more changes for this problem and switched this PR to draft now.

@kazssym
Copy link
Author

kazssym commented Feb 29, 2024 •
edited

Issue #1736

@kazssym
Revert "Rename downloaded Hugging Face model data file suffix"
38a7369 
This reverts commit c59d942.
@kazssym
Handle external data files with both _data and .data suffixes
56012bf 
This commit updates how Optimum handles external data files associated with Hugging Face models loaded from the Hub.

  - Previously: Only the `_data` suffix was checked for downloading external files.
  - Change: The code now iterates through both `"_data"` and `.data"` suffixes to ensure compatibility with models that might use either convention.

This update improves robustness by accommodating models that save external data with different suffixes.
@kazssym kazssym changed the title Correct downloadable Hugging Face model data file suffix Make optimized ONNX external data files downloadable from Hugging Face Hub Mar 3, 2024
@kazssym
Copy link
Author

kazssym commented Mar 3, 2024

Updated this PR to cope with this case:

optimum/optimum/onnxruntime/optimization.py

Line 225 in fd47a73

# TODO: ORT save_model_to_file will save as `.data` although we save as `.onnx_data` in the export

@kazssym kazssym marked this pull request as ready for review March 3, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kazssym @fxmarty