Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for the bfloat16 type to TorchSharp, fixing issue #1469 where bfloat16 tensors could not be printed. The implementation includes a new BFloat16 struct, tensor factory methods, conversion utilities, and extensive test coverage. Additionally, the PR fixes missing ToString support for Float16 types.
Changes:
- Introduced a new
BFloat16struct with IEEE 754-compliant conversion logic using round-to-nearest-even - Added tensor factory methods, Scalar conversions, and extension methods for BFloat16
- Fixed missing ToCSharpString and PrintValue handling for both BFloat16 and Float16 types
- Comprehensive test coverage including ToString, data accessors, round-trip conversions, and multidimensional tensor creation
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/TorchSharp/BFloat16.cs | New BFloat16 struct with conversion operators, arithmetic operators, and comparison operators |
| src/TorchSharp/Scalar.cs | Added BFloat16 implicit conversion and extension methods for ToScalar/ToBFloat16 |
| src/TorchSharp/Tensor/Tensor.cs | Updated ValidateType, ToCSharpString, PrintValue to handle BFloat16; added BFloat16 to type mapping; fixed Float16 ToString support |
| src/TorchSharp/Tensor/TensorExtensionMethods.cs | Added BFloat16 support to ToTensor generic methods and new ToBFloat16 extension |
| src/TorchSharp/Tensor/Factories/tensor_BFloat16.cs | Complete set of tensor factory methods for BFloat16 arrays and multidimensional arrays |
| src/TorchSharp/Tensor/Factories/as_tensor.cs | Added as_tensor overloads for BFloat16 arrays and IList |
| src/TorchSharp/Tensor/Factories/Tensor.Factories.cs | Added BFloat16 and Float16 to offset calculation in frombuffer |
| test/TorchSharpTest/TestTorchTensor.cs | Comprehensive tests for BFloat16 ToString, data access, round-trip conversion, tensor creation, struct operations, and Float16 ToString |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added BFloat16 case to PrintValue() and ToCSharpString() so that bfloat16 (and float16) tensors can be printed in all string styles.
Add test coverage for the bfloat16/float16 printing fix: - TestBFloat16ScalarToString: scalar tensor in jlstr, npstr, cstr formats - TestBFloat16TensorToString: 1-D and 2-D tensors in numpy, julia, csharp formats plus print() - TestFloat16TensorToString: 1-D tensors in csharp and numpy formats Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement a custom BFloat16 struct (2 bytes, binary-compatible with c10::BFloat16) with round-to-nearest-even conversion matching PyTorch. New features: - BFloat16 struct with arithmetic, comparison, and conversion operators - data<BFloat16>() and item<BFloat16>() for zero-copy tensor data access - torch.tensor(BFloat16[], ...) factory overloads for 1D-4D arrays - Scalar implicit conversion and ToBFloat16() extraction - ToBFloat16() tensor extension method - ToTensor<T> support for BFloat16 - as_tensor() BFloat16 overloads - frombuffer offset fix for BFloat16 and Float16 (2-byte types) Fixes dotnet#1469 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
710680f to
bd8ded6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (requires_grad && typeof(T) != typeof(float) && typeof(T) != typeof(double)) { | ||
| if (requires_grad && typeof(T) != typeof(float) && typeof(T) != typeof(double) | ||
| && typeof(T) != typeof(BFloat16)) { | ||
| throw new ArgumentException(nameof(requires_grad), "Only floating point types support gradients."); |
There was a problem hiding this comment.
The ArgumentException arguments are reversed here (constructor is ArgumentException(message, paramName)). As written, the exception message becomes "requires_grad" and the parameter name becomes the human-readable sentence, which is confusing for callers and tooling. Swap the argument order so the message explains the problem and the paramName is nameof(requires_grad).
| throw new ArgumentException(nameof(requires_grad), "Only floating point types support gradients."); | |
| throw new ArgumentException("Only floating point types support gradients.", nameof(requires_grad)); |
| if (typeof(T) == typeof(double)) | ||
| return tensor((double)(object)scalar, float64, device, requires_grad); | ||
| if (typeof(T) == typeof(BFloat16)) | ||
| return tensor(new BFloat16[] { (BFloat16)(object)scalar }, bfloat16, device, requires_grad); |
There was a problem hiding this comment.
ToTensor(scalar) for BFloat16 currently creates a 1-D length-1 tensor via the array-based tensor() overload. For consistency with the other scalar branches (byte/int/float/double), this should return a 0-D scalar tensor, otherwise APIs that expect scalar tensors (e.g., formatting/shape checks) will behave differently for BFloat16.
| return tensor(new BFloat16[] { (BFloat16)(object)scalar }, bfloat16, device, requires_grad); | |
| return tensor((BFloat16)(object)scalar, bfloat16, device, requires_grad); |
| public bool Equals(BFloat16 other) => value == other.value; | ||
| public override bool Equals(object? obj) => obj is BFloat16 other && Equals(other); | ||
| public override int GetHashCode() => value.GetHashCode(); |
There was a problem hiding this comment.
BFloat16 equality is internally inconsistent: ==/!= compare via ToSingle(), but Equals/GetHashCode compare the raw 16-bit payload. This can produce cases where (a == b) is true but a.Equals(b) is false (e.g., +0 vs -0), which breaks dictionary/set behavior and general .NET equality expectations. Align operators and Equals/GetHashCode to use the same equality definition (either bitwise or float-like) and keep them consistent.
| public bool Equals(BFloat16 other) => value == other.value; | |
| public override bool Equals(object? obj) => obj is BFloat16 other && Equals(other); | |
| public override int GetHashCode() => value.GetHashCode(); | |
| public bool Equals(BFloat16 other) => ToSingle() == other.ToSingle(); | |
| public override bool Equals(object? obj) => obj is BFloat16 other && Equals(other); | |
| public override int GetHashCode() => ToSingle().GetHashCode(); |
| public static readonly BFloat16 Epsilon = FromRawValue(0x0080); // smallest normal | ||
| public static readonly BFloat16 SmallestSubnormal = FromRawValue(0x0001); |
There was a problem hiding this comment.
The constant named Epsilon is documented/implemented as “smallest normal” (0x0080 == 2^-126). In .NET numeric types, Epsilon typically means the smallest positive value > 0 (usually the smallest subnormal). Since you already expose SmallestSubnormal, consider either redefining Epsilon to match .NET expectations or renaming this constant to something like MinNormal to avoid surprising API behavior.
| public static readonly BFloat16 Epsilon = FromRawValue(0x0080); // smallest normal | |
| public static readonly BFloat16 SmallestSubnormal = FromRawValue(0x0001); | |
| public static readonly BFloat16 SmallestSubnormal = FromRawValue(0x0001); // smallest positive (subnormal) | |
| public static readonly BFloat16 Epsilon = SmallestSubnormal; // .NET-style epsilon: smallest positive > 0 | |
| public static readonly BFloat16 MinNormal = FromRawValue(0x0080); // smallest normal |
Fixes #1469