Skip to content

Explain what "normal" is in Double.IsNormal#12281

Open
sfiruch wants to merge 1 commit intodotnet:mainfrom
sfiruch:patch-2
Open

Explain what "normal" is in Double.IsNormal#12281
sfiruch wants to merge 1 commit intodotnet:mainfrom
sfiruch:patch-2

Conversation

@sfiruch
Copy link

@sfiruch sfiruch commented Feb 4, 2026

Summary

The documentation for Double.IsNormal is wholly unhelpful. It says it checks if a number is "Normal". Only in the remarks, "normal" is explained. This changes it to make it clear what it actually checks.

@sfiruch sfiruch requested a review from a team as a code owner February 4, 2026 10:01
Copilot AI review requested due to automatic review settings February 4, 2026 10:01
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 4, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the API documentation for Double.IsNormal to clarify what “normal” means directly in the summary text.

Changes:

  • Replaces the IsNormal <summary> text with an explicit definition (not NaN, not infinite, not subnormal, and not zero).

Comment on lines 2952 to 2953
<see langword="true" /> if the value is normal; <see langword="false" /> otherwise.</returns>
<remarks>This effectively checks the value is not NaN, not infinite, not subnormal, and not zero.</remarks>
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The updated

explains what “normal” means, but the text still relies on the term “normal” without the definition. Consider updating to use the same explicit criteria (or add the definition there) so readers don’t have to infer it from the summary/remarks.

Also, the criteria list is now repeated in both

and , which creates duplication that can drift over time. Consider removing the redundant list from or using for additional context instead of repeating the same definition.

Suggested change
<see langword="true" /> if the value is normal; <see langword="false" /> otherwise.</returns>
<remarks>This effectively checks the value is not NaN, not infinite, not subnormal, and not zero.</remarks>
<see langword="true" /> if the value is not NaN, not infinite, not subnormal, and not zero; <see langword="false" /> otherwise.</returns>
<remarks>Use this method to determine whether a double-precision floating-point number is a normal value before performing calculations that assume finite, nonzero magnitudes.</remarks>

Copilot uses AI. Check for mistakes.
<Docs>
<param name="d">A double-precision floating-point number.</param>
<summary>Determines whether the specified value is normal.</summary>
<summary>Determines whether the specified value is not NaN, not infinite, not subnormal, and not zero.</summary>
Copy link
Member

Choose a reason for hiding this comment

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

If you're wanting to not be repetitive, then repeating subnormal doesn't really help either.. The technical definition is:

Suggested change
<summary>Determines whether the specified value is not NaN, not infinite, not subnormal, and not zero.</summary>
<summary>Determines whether the specified value is a finite non-zero value with magnitude greater than or equal to <c>2<sup>-1022</sup></c>.</summary>

Copy link
Member

@tannergooding tannergooding Feb 4, 2026

Choose a reason for hiding this comment

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

I would expect Single, Half, NFloat, and INumberBase<TSelf> to all be updated in the same PR.

I would also expect other definitions like IsInfinite, IsSubnormal, IsFinite, etc to be updated if required.

Copy link
Member

Choose a reason for hiding this comment

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

And as per the Copilot guidance below, the summary and remarks are effectively just mirroring eachother now, so the remarks need to either be removed or updated.

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

Labels

area-System.Runtime community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants