Skip to content

Conversation

gewarren
Copy link
Contributor

@gewarren gewarren commented Apr 20, 2021

Fixes #23030. (Hide whitespace changes.)

See dotnet/runtime#37630 and dotnet/runtime#936.

Internal preview.

@gewarren gewarren requested a review from adegeo as a code owner April 20, 2021 22:52
@dotnet-bot dotnet-bot added this to the April 2021 milestone Apr 20, 2021
|<xref:System.Int16>|16-bit signed integer|
|<xref:System.Int32>|32-bit signed integer|
|<xref:System.Int64>|64-bit signed integer|
|<xref:System.Half>|Half-precision floating-point value|
Copy link
Contributor

@adegeo adegeo Apr 21, 2021

Choose a reason for hiding this comment

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

It may be worth pointing out what is said in the API docs:

An IEEE 754 compliant float16 type.

Copy link
Member

Choose a reason for hiding this comment

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

The formal IEEE 754 name is binary16 for half, binary32 for single, and binary64 for double.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tannergooding we should probably update the API docs, as it says float16: https://docs.microsoft.com/en-us/dotnet/api/system.half?view=net-5.0

@gewarren gewarren requested a review from tannergooding April 21, 2021 21:59

|Type|Size (in bytes)|Approximate range|Precision|Notes|
|----------|--------|---------------------|--------------------|
|<xref:System.Half?displayProperty=nameWithType>|2|||Introduced in .NET 5|
Copy link
Member

@tannergooding tannergooding Apr 21, 2021

Choose a reason for hiding this comment

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

The exact range for Half is +/-65504.

The number of digits, mirroring what is used in the other columns is ~3-5 digits. However, that column is very misleading in my opinion and may give users a false sense that any value with 6 significant digits can be exactly represented.

In practice, the lower limit here is that if a string with at most n significant digits (3, 6, 15 for half, float, and double, respectively) is converted to a floating-point value and then back to a string with the same number of digits, the strings should match. That is, float.Parse(s).ToString("G6") == s should be true for any s that contains no more than 6 significant digits.

  • This isn't guaranteed by the IEEE 754 spec and may depend on the formatting algorithm used

While the upper limit here is that if a floating-point value is converted to a string with that many digits (5, 9, 17 for half, float, and double, respectively), and then converted back to a floating-point value, the result should match the original floating-point value. That is, float.Parse(x.ToString("G9")).Equals(x) should be true for any float.

  • This is guaranteed by the IEEE 754 spec, and we should be correctly implementing this in .NET Core 3.1 or higher

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will plan to remove that column then. Thanks!

Copy link

@pgovind pgovind left a comment

Choose a reason for hiding this comment

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

Other reviews have called out everything I noticed too. Approving assuming the comments will be resolved! Overall LGTM!

@gewarren gewarren requested a review from a team as a code owner April 22, 2021 22:11
@gewarren gewarren closed this Apr 23, 2021
@gewarren gewarren reopened this Apr 23, 2021
@gewarren gewarren merged commit 63e9b2d into dotnet:main Apr 23, 2021
@gewarren gewarren deleted the half branch April 23, 2021 03:50
@gewarren
Copy link
Contributor Author

Thanks for the great reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need to add Half to this page

6 participants