-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[ntuple] Add support for half-precision floats #13844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/python3. |
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Errors:
|
|
Build failed on ROOT-performance-centos8-multicore/soversion. Errors:
|
|
Build failed on windows10/default. Errors:
And 1 more |
|
Build failed on mac11/noimt. |
a3011fe to
d5a668d
Compare
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/soversion. Errors:
|
|
Build failed on mac12arm/cxx20. Errors:
|
|
Build failed on ROOT-ubuntu2004/python3. Errors:
|
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Errors:
|
|
Build failed on windows10/default. Errors:
|
d5a668d to
c08b11a
Compare
|
Starting build on |
|
Build failed on windows10/default. Errors:
|
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Failing tests: |
c08b11a to
047aded
Compare
|
Starting build on |
|
Build failed on windows10/default. Errors:
|
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Failing tests: |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
047aded to
d11e9df
Compare
|
Starting build on |
jblomer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! I think it's very good shape.
Let's also add the new column type to the Packing.OnDiskEncoding unit test.
| #define HALF_ENABLE_F16C_INTRINSICS __F16C__ | ||
| #endif | ||
| #if HALF_ENABLE_F16C_INTRINSICS | ||
| #include <immintrin.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, does this actually enable intrinsics on a "standard platform"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not by default. ROOT would have to be compiled with -mf16c, but this doesn't seem to play nice with projectroot.test.test_TFormulaTests when added to the CMake configuration. It might be worth it to further investigate and come up with a solution, though.
tree/ntuple/v7/test/ntuple_types.cxx
Outdated
| FileRaii fileGuard("test_ntuple_float16.root"); | ||
|
|
||
| auto f1Fld = RFieldBase::Create("f1", "float").Unwrap(); | ||
| f1Fld->SetColumnRepresentative({ROOT::Experimental::EColumnType::kReal16}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| f1Fld->SetColumnRepresentative({ROOT::Experimental::EColumnType::kReal16}); | |
| f1Fld->SetHalfPrecision(); |
d11e9df to
0ed771b
Compare
|
Starting build on |
0ed771b to
d924406
Compare
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
jblomer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Let's add one more test to the Packing, OnDiskEncoding unit test in ntuple_packing to verify the on-disk bit pattern is as expected.
| floatArray[i] = Internal::HalfToFloat(uint16Array[i]); | ||
| ByteSwapIfNecessary(floatArray[i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these two lines be reversed (in reverse order to packing)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're completely right!
d924406 to
fefd73a
Compare
|
Starting build on |
This PR adds support to 16-bit (IEEE 754-2008) floating point numbers to RNTuple. In similar vein to 32-bit doubles, this is enabled by changing the column representation of ordinary
floatfields (i.e., regularfloatfields are used, but the values are saved to disk using half precision).Because half-precision floats are only part of the C++ standard from C++23 onwards, for now we use parts of the
halflibrary (MIT-licensed) for converting between half- and single-precision floats. For x86 processors, the F16C ISA extension can be enabled to do this conversion on the hardware directly. However, this appears to mess withTFormulaTests.