Skip to content

fix(testing): support fp16 and float8_e5m2 in dtype_to_str - #29

Open
LeonxLJX wants to merge 2 commits into
deepseek-ai:mainfrom
LeonxLJX:fix-dtype-to-str-fp16-e5m2
Open

fix(testing): support fp16 and float8_e5m2 in dtype_to_str#29
LeonxLJX wants to merge 2 commits into
deepseek-ai:mainfrom
LeonxLJX:fix-dtype-to-str-fp16-e5m2

Conversation

@LeonxLJX

@LeonxLJX LeonxLJX commented Sep 1, 2026

Copy link
Copy Markdown

Summary

tile_kernels/testing/bench.py::dtype_to_str() raised ValueError for torch.float16 and torch.float8_e5m2, even though TileKernels' quant kernels use these dtypes in benchmarks.

Changes

  • Add torch.float16: 'fp16' and torch.float8_e5m2: 'e5m2' to the mapping.
  • Update the error message to list the newly supported dtypes.

Fixes #4. Claimed via @LeonxLJX.

Comment thread tile_kernels/testing/bench.py Outdated

if dtype not in mapping:
raise ValueError(f'Unsupported dtype: {dtype}. Only fp32, bf16, e4m3, and int8(e2m1) are supported')
raise ValueError(f'Unsupported dtype: {dtype}. Only fp32, fp16, bf16, e4m3, e5m2, and int8(e2m1) are supported')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔵 suggestion: 可选改进:错误消息中的支持列表可用 ', '.join(mapping.values()) 动态生成,避免未来新增 dtype 时映射与消息不同步。非阻塞。

🤖 v5

@ds-review-bot

Copy link
Copy Markdown
Collaborator

🤖 ds-review-bot Code Review

v6

变更直接补充了两个目标 dtype 的映射,并同步更新错误信息,未发现会破坏现有行为的问题。

v5

该 MR 修复了 tile_kernels/testing/bench.py 中 dtype_to_str() 不支持 torch.float16 和 torch.float8_e5m2 的问题(Fixes #4)。审查确认:mapping 中已正确加入 torch.float16 -> 'fp16'(第62行)和 torch.float8_e5m2 -> 'e5m2'(第65行),命名与现有 fp32/bf16/e4m3 风格一致;第70行错误消息也已同步更新为列出 fp32, fp16, bf16, e4m3, e5m2, int8(e2m1)。改动最小、正确且与描述一致,多个 benchmark 测试(quant/transpose/moe)依赖该函数,此修复消除了 fp16/e5m2 下的 ValueError。建议合并。

v4p

本 MR 针对 tile_kernels/testing/bench.py 的 dtype_to_str(),为 torch.float16 和 torch.float8_e5m2 增加字符串映射,并同步更新不支持 dtype 的报错信息,避免量化内核基准测试中这些 dtype 触发 ValueError。变更范围小、实现直接,新增映射的命名与既有 e4m3/bf16 约定一致;当前调用方仅将该函数用于测试 ID 与 benchmark 参数记录,未发现正确性问题。

Files reviewed: 1
Issues found: 🔵 1 suggestion
Inline comments posted: 1

@LeonxLJX

LeonxLJX commented Sep 1, 2026

Copy link
Copy Markdown
Author

Thanks @ds-review-bot for the review! Applied the suggestion — the supported-dtype list in the error message is now generated dynamically via ', '.join(mapping.values()) to avoid drift. (via @LeonxLJX)

@ds-review-bot

Copy link
Copy Markdown
Collaborator

⚠️ 抱歉,只有对该仓库有 Write 权限的用户才能触发代码审查。

@LeonxLJX

LeonxLJX commented Sep 1, 2026

Copy link
Copy Markdown
Author

Root cause. tile_kernels/testing/bench.py's dtype_to_str only mapped a subset of torch dtypes, so fp16 and float8_e5m2 (the latter now used by several quantized / bench paths) fell through to the unhandled branch — the benchmark either errored or silently mislabelled the dtype.

My fix. Extended the mapping with the exact canonical torch type strings (torch.float16'fp16', torch.float8_e5m2'e5m2'), keeping the existing single-space formatting the lookup expects so the string match stays correct.

Suggestion for maintainers (up for debate). This mapping is exactly the kind of thing that silently drifts every time torch adds a dtype. A more robust fix is to derive the string from a small registry / torch.typename instead of a hand-maintained dict, or at minimum add a test asserting dtype_to_str resolves every dtype torch.finfo / torch.iinfo knows about, so a future torch release fails loudly instead of mislabelling. Happy to add that test in a follow-up PR if it's welcome.

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.

dtype_to_str: unsupported dtype raises ValueError for fp16/float8_e5m2

2 participants