Skip to content

Fix dtype_byte_size reporting zero bytes for sub-byte and packed dtypes - #4237

Open
VihaanAgarwal wants to merge 1 commit into
huggingface:mainfrom
VihaanAgarwal:dtype-byte-size-subbyte
Open

Fix dtype_byte_size reporting zero bytes for sub-byte and packed dtypes#4237
VihaanAgarwal wants to merge 1 commit into
huggingface:mainfrom
VihaanAgarwal:dtype-byte-size-subbyte

Conversation

@VihaanAgarwal

Copy link
Copy Markdown

What does this PR do?

dtype_byte_size falls back to reading the trailing digits of the dtype name:

bit_search = re.search(r"[^\d](\d+)$", str(dtype))
return int(bit_search.groups()[0]) // 8

For torch's sub-byte and packed dtypes that trailing number is not the storage size, and the integer division turns it into zero:

uint4              -> 0   (torch stores one element per byte)
uint1              -> 0
float4_e2m1fn_x2   -> 0   (two values packed in one byte, itemsize 1)

compute_module_sizes and therefore infer_auto_device_map then count such parameters as free, so a device map can overcommit a device by the whole size of those weights.

torch has exposed dtype.itemsize since 2.1, the same version that already gates the FP8 branch. This PR returns itemsize under that gate, which also replaces the hardcoded FP8 name list. The regex stays as the fallback for torch 2.0. torch.bool and the CustomDtype values keep their explicit sizes.

The float8_e8m0fnu and sub-byte cases in test_dtype_byte_size fail on main with 0 != 1 and pass with the change. tests/test_modeling_utils.py passes locally (torch 2.14, CPU).

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? No change needed.
  • Did you write any new necessary tests?

Who can review?

@SunMarc

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.

1 participant