Skip to content

Conversation

huoyaoyuan
Copy link
Member

Closes #96295.

Delegating all functional members to float, since the upcast is trivial. The only logic is rounding from double.
Tests are borrowed from Half.

@ghost
Copy link

ghost commented Feb 19, 2024

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Feb 19, 2024

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Closes #96295.

Delegating all functional members to float, since the upcast is trivial. The only logic is rounding from double.
Tests are borrowed from Half.

Author: huoyaoyuan
Assignees: -
Labels:

area-System.Numerics, new-api-needs-documentation

Milestone: -

@huoyaoyuan huoyaoyuan added the community-contribution Indicates that the PR has been added by a community member label Feb 19, 2024
@tannergooding
Copy link
Member

The changes generally LGTM. However there are still some open comments/feedback that should be addressed (at least getting a response elaborating why it doesn't need to be handled) before this can be merged

@huoyaoyuan
Copy link
Member Author

I should have addressed all comments, and log rounding concerns with other FP types to #112474.

Interaction between BFloat16<->NFloat in not included because I'm not sure where to deal with it.

For Complex and BigInteger, actually I think it needs to reconsider whether create interactions in IBinaryFloatingPointIeee754.

@tannergooding
Copy link
Member

Interaction between BFloat16<->NFloat in not included because I'm not sure where to deal with it.

This one is fine to split between them or to put it entirely in NFloat since its the less used type.

For Complex and BigInteger, actually I think it needs to reconsider whether create interactions in IBinaryFloatingPointIeee754

Not sure what you mean? This one should be entirely in Complex/BigInteger due to layering.

@huoyaoyuan
Copy link
Member Author

huoyaoyuan commented Sep 24, 2025

For Complex and BigInteger, actually I think it needs to reconsider whether create interactions in IBinaryFloatingPointIeee754

Not sure what you mean? This one should be entirely in Complex/BigInteger due to layering.

I'm concerned about whether they should take a dependency on BFloat16 at all, since neither of them are "core" type. There's no dependency between BigInteger and NFloat because they are for different areas.
If we can define conversion operator in the new extension block, we can define "attatched" conversion avoid introducing dependency between them.

The relationship between other types appears to be not included in the original API review. I really suggest to avoid introducing more dependency ship for now. We may add them later.

@tannergooding
Copy link
Member

I'm concerned about whether they should take a dependency on BFloat16 at all, since neither of them are "core" type.

BFloat16 is a hardware accelerated "built-in" that will end up being a core part of ML/AI for a long time. It's not as core as others, but it's still something that's desirable to support.

There's no dependency between BigInteger and NFloat because they are for different areas.

This would rather mainly be due to oversight. NFloat is frequently forgotten about and is mainly just for interop, rather than as a general purpose type usable by any managed code.

If we can define conversion operator in the new extension block, we can define "attatched" conversion avoid introducing dependency between them.

Conversion operators are not usable in generic contexts like that, that is the reason we have CreateChecked, CreateSaturating, and CreateTruncating with their shape setup as it is.

The relationship between other types appears to be not included in the original API review. I really suggest to avoid introducing more dependency ship for now. We may add them later.

There is no additional "dependency". There is simply that we have a set of types that we ship "in box" and we want them to generally all work between eachother. That is part of the core and fundamental design of generic math.

We can always provide a fallback that tries to deal with things generically, but that is expensive and rarely needed in practice since we can trivially and more cheaply just directly handle the known types.

@huoyaoyuan
Copy link
Member Author

huoyaoyuan commented Sep 26, 2025

Is there anything to be added before being ready to merge now?

@tannergooding tannergooding merged commit f00121f into dotnet:main Oct 2, 2025
144 checks passed
@huoyaoyuan huoyaoyuan deleted the BFloat16 branch October 2, 2025 17:02
@xtqqczze
Copy link
Contributor

xtqqczze commented Oct 4, 2025

Do we want to have performance benchmarks for this type, like https://github.com/dotnet/performance/blob/main/src/benchmarks/micro/libraries/System.Runtime/Perf.Half.cs?

Opened dotnet/performance#5002 for discussion.

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

Labels

area-System.Numerics community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal]: BFloat16

9 participants