Skip to content

Conversation

@Cyrilvallez
Copy link
Member

What does this PR do?

As per the title. This upstream the change made to the space converter https://huggingface.co/spaces/safetensors/convert/discussions/41.
Related to huggingface/transformers#41038 and huggingface/transformers#38870 for the full picture

@daskol
Copy link
Contributor

daskol commented Sep 23, 2025

Could you move convert.py into the safetensors package? It will make convertation to safetensors with safetensors more user-friendly.

Also, this script may not use huggingface_hub at all! Basically, it works with files a directory on a local filesystem. That is the common scenario in private clouds which often lack Internet access. Moreover, convert.py should be work more carefully with temporary directories (e.g. it can be located on a volume of limited space or it does not exists at all).

@Narsil
Copy link
Contributor

Narsil commented Sep 27, 2025

Could you move convert.py into the safetensors package? It will make convertation to safetensors with safetensors more user-friendly.

Please no. This script is way too complex for most cases as it tries to solve transformers specific cases. And simple torch.load safetensors.torch.save_file should be enough in most cases (with save_model enabling almost every other use case).

convert.py is in the source code so it's readable and can provide help for power users, but shouldn't be distributed in the distributed code imho because it has knowingly some gaps and specific choices which don't work in general.

@daskol
Copy link
Contributor

daskol commented Sep 27, 2025

Maybe, a better option is to put this convert.py to transformers (or huggingface-hub) then?

What about fixing "some gaps and specific choices" in general? As you previously said, there are some subtle points and issues in seriralization/deserialization (and this PR exists due to huggingface/transformers#41038) so one could expect these discrepancies will be solved eventually. But, based on your comment above, you’re fine with the current state of things.

@Cyrilvallez
Copy link
Member Author

Cyrilvallez commented Sep 29, 2025

I agree with @Narsil that there is no reason to distribute this in any library. It's a tool that was created to help integrating different HF libraries together, but does not really belong to any of them. TBH, I thought it was only a script on the space here and did not now it lived here as well - I'm just upstreaming here now for the sake of completeness as the other one was updated.
In general, if you want to make this tool more general to work with local folders etc, feel free to reuse it and adapt it to your own needs!

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