-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add cpu accelerator fp16 dtype support #7207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Lai, Yejing <[email protected]>
accelerator/cpu_accelerator.py
Outdated
@@ -232,7 +232,7 @@ def is_fp16_supported(self): | |||
return False | |||
|
|||
def supported_dtypes(self): | |||
return [torch.float, torch.bfloat16] | |||
return [torch.float, torch.bfloat16, torch.float16] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yejing-Lai - perhaps this is the runner, but the default we have here does not have torch.float16 support (see unit test errors).
I think this may come back to the discussions we've had before on splitting the cpu accelerator into a cpu accelerator (generic cpu) and a xeon accelerator (PR here: #5126 )?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loadams, good point.
@Yejing-Lai, are you specifically targetting xeon with this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I test this PR on xeon and it works fine. We will discuss determining the datatype list based on the CPU flags. Please pending this PR temporarily, thanks~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to keep one CPU accelerator with different features. Xeon is not much different from perspective of Pytorch CPU support. Desktop or other non-Xeon CPU might also get fp16 support if they implemented proper instruction set and get supported in Pytorch. So it should be reasonable to have one CPU accelerator instead of seperate xeon accelerator.
I think a proper way is to detect whether the CPU+PyTorch has ability to support FP16 datatype, this might be more resilient. @Yejing-Lai is there a way to detect whether PyTorch support fp16 on CPU or not, from Intel Extention for PyTorch team feedback? |
Signed-off-by: Lai, Yejing <[email protected]>
Head branch was pushed to by a user without write access
Add cpu accelerator fp16 dtype support