Skip to content

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

Merged
yihonglyu merged 8 commits intomainfrom
yilyu/fix-fp16-dw-conv-bias
Jul 30, 2024
Merged

Enable FP16 Clip and Handle Bias in FP16 Depthwise Conv#21493
yihonglyu merged 8 commits intomainfrom
yilyu/fix-fp16-dw-conv-bias

Conversation

@yihonglyu
Copy link
Copy Markdown
Contributor

@yihonglyu yihonglyu commented Jul 25, 2024

@yihonglyu yihonglyu requested a review from a team as a code owner July 25, 2024 02:31
Comment thread onnxruntime/core/mlas/inc/mlas.h
Comment thread onnxruntime/core/mlas/lib/dwconv.cpp Outdated
@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
Comment thread onnxruntime/core/mlas/lib/fp16_common.h
@yihonglyu yihonglyu requested a review from edgchen1 July 29, 2024 01:30
Copy link
Copy Markdown
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

Comment thread onnxruntime/core/mlas/lib/fp16_common.h
@yihonglyu yihonglyu merged commit 530a2d7 into main Jul 30, 2024
@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.
rohan11235813 pushed a commit to quadric-io/onnxruntime that referenced this pull request Aug 19, 2025
### 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
microsoft/onnxruntime#21957 introduced by
microsoft/onnxruntime#21493.
rohan11235813 pushed a commit to quadric-io/onnxruntime that referenced this pull request Sep 15, 2025
### 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
microsoft/onnxruntime#21957 introduced by
microsoft/onnxruntime#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