Skip to content
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

Enable FP16 Clip and Handle Bias in FP16 Depthwise Conv #21493

Merged
merged 8 commits into from
Jul 30, 2024

Conversation

yihonglyu
Copy link
Contributor

@yihonglyu yihonglyu commented Jul 25, 2024

@yihonglyu yihonglyu requested a review from a team as a code owner July 25, 2024 02:31
onnxruntime/core/mlas/inc/mlas.h Show resolved Hide resolved
onnxruntime/core/mlas/lib/dwconv.cpp Outdated Show resolved Hide resolved
@yihonglyu yihonglyu changed the title Enable FP16 Clip and Fix Bias bug in FP16 Depthwise Conv Enable FP16 Clip and Handle Bias in FP16 Depthwise Conv Jul 25, 2024
@yihonglyu yihonglyu requested a review from edgchen1 July 27, 2024 05:28
@yihonglyu yihonglyu requested a review from edgchen1 July 29, 2024 01:30
Copy link
Contributor

@chenfucn chenfucn left a comment

Choose a reason for hiding this comment

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

look good to me

onnxruntime/core/mlas/lib/fp16_common.h Show resolved Hide resolved
@yihonglyu yihonglyu merged commit 530a2d7 into main Jul 30, 2024
95 checks passed
@yihonglyu yihonglyu deleted the yilyu/fix-fp16-dw-conv-bias branch July 30, 2024 10:49
tianleiwu added a commit that referenced this pull request Sep 26, 2024
### Description
* Add std::numeric_limits for MLFloat16 and BFloat16.
* Update some comments in csharp ORTFloat16.shared.cs.
* Add unit tests (including Clip)

Note that the canonical NaN is not consistent in C++ and C#. C# uses
negative quiet NaN as canonical NaN, while C++ uses positive quiet NaN.
The choice of CSharp Float16.NaN is to be consistent with
System.Half.NaN.

FP16 data returns from CUDA might have 7FFF as NaN; FP16 data from CPU
provider might have 0x7E00 as NaN. Anyway there is no consistent
canonical NaN in ORT right now. Because all these NaNs are aligned with
IEEE spec, there shall not an issue in downstream.

### Motivation and Context
std::numeric_limits is used in codebase but not defined for MLFloat16
and BFloat16. It causes some bugs like
#21957 introduced by
#21493.
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.

3 participants