huggingface / huggingface/transformers

[model loading] framework-agnostic dtype parameter

Open
#13,246 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

WIP
Dominant language
Python
Stars
166k
Forks
34.6k
Avg merge
3d 9h
Merged PRs (30d)
281

Description

This is a split off from one of the discussions at https://github.com/huggingface/transformers/pull/13209:

  1. It all started with trying to load torch models under either the desired dtype or the the dtype of the pretrained model - and thus avoid 2x memory usage needs e.g. if the model needs to be just fp16. So we added torch_dtype to from_pretrained and from_config.
  2. Then we started storing torch_dtype in the config file for future possibly automatic loading model in the optimal "regime".
  3. This resulted in a discrepancy where the same symbol sometimes means torch.dtype at other times a string like "float32" as we can't store torch.dtype in json.
  4. then in https://github.com/huggingface/transformers/pull/13209#discussion_r693292542 we started discussing how dtype is really the same across pt/tf/flux and perhaps we should just use dtype in the config and variables and have it consistently to be a string ("float32") and convert it to the right dtype object of the desired framework at the point of use, e.g. getattr(torch, "float32")

A possible solution is to deprecate torch_dtype and replace it with dtype string both in config and in the function argument.

Possible conflicts with the naming:

  1. we already have the dtype attribute in modeling_utils, which returns torch.dtype based on the first param's dtype.

    https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_utils.py#L205

    The context is different, but still this is something to consider to avoid ambiguity.

I may have missed some other areas. So please share if something else needs to be added.

Additional notes:

#13098 - the idea of the PR is exactly to disentangle parameter dtype from matmul/computation dtype. In Flax, it's common practice that the dtype parameter defines the matmul/computation dtype, see: https://flax.readthedocs.io/en/latest/_autosummary/flax.linen.Dense.html#flax.linen.Dense.dtype instead of the parameter dtype and not the parameter dtype.
So for Flax, I don't really think it would make sense to use a config.dtype to define weights dtype as it would be quite confusing with Flax's computation dtype parameter.

@LysandreJik, @sgugger, @patrickvonplaten

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing modeling_utils.py, the dtype attribute, and the from_pretrained and from_config entry points, then read the linked discussion in pull request 13209. Trace how torch_dtype is represented in configuration and converted for each framework. Done means the naming and compatibility approach is agreed and the affected loading paths are consistently covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, tensorflow
Domain
machine-learning
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.