Add FP8 placeholder support to ExecuTorch serialization#19043
Conversation
Change-Id: Ibb7ef4167ab96426133fce64e34366c365cd12ad Signed-off-by: Yufeng Shi <yufeng.shi@arm.com>
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19043
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ No FailuresAs of commit 753c9d2 with merge base 5eb8492 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hey @digantdesai, could you take a look at this RP? Thank you |
|
@claude review this PR. |
|
Claude finished @digantdesai's task in 3m 52s —— View job Review: Add FP8 placeholder support to ExecuTorch serialization
SummaryThis PR adds
Review FeedbackOverall: Looks good. The changes are straightforward and correctly integrate with the two separate ScalarType enums (the serde schema's own numbering and the runtime's c10-compatible numbering). A few minor observations: 1. Serde enum ordering (non-blocking, but worth noting)In 2. Missing trailing comma (nit)In 3. Test coverage is appropriateThe test creates FP8 tensors via 4. Consideration: should
|
digantdesai
left a comment
There was a problem hiding this comment.
Thanks. Any plans to further support this down the lowering pipeline in ET?
digantdesai
left a comment
There was a problem hiding this comment.
Thanks. Any plans to further support this down the lowering pipeline in ET?
digantdesai
left a comment
There was a problem hiding this comment.
Thanks. Any plans to further support this down the lowering pipeline in ET?
Yes. I'll start working on FP8 operator support for the Arm backend after this patch lands. |
Change-Id: Ibb7ef4167ab96426133fce64e34366c365cd12ad
cc @JacobSzwejbka @angelayi