Initial implementation of float16 support #10342

eschnett · 2024-06-30T17:11:58Z

This implements support for the float16 datatype as proposed in #10146.

rouault

Thanks for tackling this. One major adjustment will be to introduce #ifdef at all places where we reference _Float16

There will be also the question on how to deal with Float16 datasets on environments where this is not a native data type. We should probably continue to expose them as Float32 as done for example by the GTiff, HDF5 or Zarr driver in current master. Otherwise we'll have to make sure that RasterIO() / GDALCopyWords() can work for Float16 <--> Float32 conversions even if _Float16 is not a native data type (using the functions of port/cpl_float.h)
Same issue with the VRT driver: what if a user creates/reads a VRT file referencing Float16 on a system without _Float16 support.

If you could rebase on top of latest master to solve a conflict, that would enable to have CI to trigger

rouault · 2024-06-30T20:38:39Z

gcore/gdal_priv.h

 }

+// The generic function above does not work for _Float16 because
+// std::numeric_limits<float>::epsilon() is the wrong choice
+#ifndef SWIGPYTHON


Why testing for SWIGPYTHON outside of swig/include? That doesn't seem appropriate

SWIG examines this file and is getting confused because it does not know _Float16. Is there a different way to tell SWIG to ignore this function? (I'm not a SWIG expert.)

ok I see. Perhaps use #ifndef SWIG to be more general then

The error is

In file included from extensions/gdal_array_wrap.cpp:3395: /localhome/eschnett/src/gdal/gcore/gdal_priv.h:4414:28: error: ‘bool ARE_REAL_EQUAL’ redeclared as different kind of entity 4414 | inline bool ARE_REAL_EQUAL(_Float16 hfVal1, _Float16 hfVal2, int ulp = 2) | ^~~~~~~~

rouault · 2024-06-30T20:39:09Z

gcore/gdalmultidim.cpp

@@ -1692,6 +1695,9 @@ bool GDALExtendedDataType::CopyValue(const void *pSrc,
                                 static_cast<GIntBig>(
                                     *static_cast<const std::int64_t *>(pSrc)));
                break;
+            case GDT_Float16:
+                str = CPLSPrintf("%.9g", *static_cast<const _Float16 *>(pSrc));


I would expect float16 requires less than 9 significant digits. Same below for CFloat16

Thanks, it requires 5 digits.

I notice that double values are output with 18 digits. 17 would suffice.

rouault · 2024-06-30T20:43:48Z

frmts/zarr/zarr_v2_array.cpp

@@ -1155,9 +1155,11 @@ static GDALExtendedDataType ParseDtype(const CPLJSONObject &obj,
            }
            else if (chType == 'f' && nBytes == 2)
            {
+                // elt.nativeType = DtypeElt::NativeType::IEEEFP;


exposing has Float32 will still be needed for environments without native Float16 support

rouault · 2024-06-30T20:46:42Z

frmts/gtiff/gt_overview.cpp

@@ -375,6 +375,10 @@ CPLErr GTIFFBuildOverviewsEx(const char *pszFilename, int nBands,
                nBandFormat = SAMPLEFORMAT_INT;
                break;

+            case GDT_Float16:
+                CPLAssert(false);


The assertion is inappropriate, as this could happen. We should error out with a proper CPLError(CE_Failure, CPLE_NotSupported, ...) error. Same remark for CFloat16 below. Actually we could potentially support Float16. There's for example support for it in frmts/gtiff/gtiffoddbitsband.cpp

rouault · 2024-06-30T20:48:06Z

frmts/vrt/vrtderivedrasterband.cpp

@@ -105,6 +105,9 @@ static PyObject *GDALCreateNumpyArray(PyObject *pCreateArray, void *pBuffer,
        case GDT_UInt64:
            pszDataType = "uint64";
            break;
+        case GDT_Float16:
+            CPLAssert(FALSE);


this assert should be converted to a CPLError(), or implemented

rouault · 2024-06-30T20:51:00Z

frmts/vrt/pixelfunctions.cpp

@@ -59,6 +59,8 @@ inline double GetSrcVal(const void *pSource, GDALDataType eSrcType, T ii)
        case GDT_Int64:
            return static_cast<double>(
                static_cast<const int64_t *>(pSource)[ii]);
+        case GDT_Float16:


The data type enumeration in frmts/vrt/data/gdalvrt.xsd will need to be extended

Done. I assume you refer to DataTypeType. I assume the datatypes there are essentially unordered and do not have to have the same order as enum GDALDataType. I also added Int8 which seemed to be missing.

eschnett · 2024-07-09T18:12:52Z

I am planning to use #ifdef SIZEOF__FLOAT16 to check whether _Float16 is available. Would you prefer a different mechanism?

rouault · 2024-07-09T18:24:02Z

I am planning to use #ifdef SIZEOF__FLOAT16 to check whether _Float16 is available.

that sounds reasonable. We should make sure that this is set in cmake/template/cpl_config.h.in so that code like out-of-tree drivers has also access to that define when building against GDAL headers

eschnett · 2024-07-09T19:45:16Z

Should GetSrcVal in pixelfunctions.cpp return 0 for float16, or report an error if unsupported?

…eschnett/gdal into eschnett/float16-implementation

github-actions · 2024-08-27T02:41:55Z

The GDAL project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 28 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular link
to any issues which this pull request fixes
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in 2 weeks.

github-actions · 2024-09-10T02:44:18Z

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 6 weeks. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the GDAL project can do to help push this PR forward please let us know how we can assist.

Initial implementation of float16 support

560a333

rouault reviewed Jun 30, 2024

View reviewed changes

eschnett added 2 commits July 9, 2024 15:20

Define and use SIZEOF__FLOAT16

cb57004

Merge branch 'master' into eschnett/float16-implementation

3390e9a

eschnett added 19 commits July 9, 2024 15:46

Correct various errors

7fd149b

Clean up GDALRasterBand statistics

5662f6f

Add more _Float16 #ifdefs

54e82c8

Slightly more float16 cleanup

d6c2484

zarr: Add more types to DMD_CREATIONDATATYPES

1ee5414

Avoid compiler warnings

b551080

Correct some test cases

fd91cec

Correct test case

20ee002

Merge branch 'master' into eschnett/float16-implementation

6d96613

Reformat code

f5d3e99

Reformat code

26431c2

Make quicktest pass

eedd958

Introduce GDALNumericLimits

7d28b1c

Avoid compiler warning

d6d7ad2

Avoid compiler warning

a92f5cb

Fix more bugs

4cf170e

Improve error messages

a86d23a

Disable Zarr Python Float16 tests

5961f90

Merge branch 'eschnett/float16-implementation' of https://github.com/…

b92c788

…eschnett/gdal into eschnett/float16-implementation

github-actions bot added the stale label Aug 27, 2024

github-actions bot closed this Sep 10, 2024

eschnett mentioned this pull request Oct 30, 2024

Initial implementation of float16 support #11180

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial implementation of float16 support #10342

Initial implementation of float16 support #10342

eschnett commented Jun 30, 2024

rouault left a comment

rouault Jun 30, 2024

eschnett Jul 9, 2024

rouault Jul 9, 2024

eschnett Jul 9, 2024

rouault Jun 30, 2024

eschnett Jul 9, 2024

rouault Jun 30, 2024

rouault Jun 30, 2024

rouault Jun 30, 2024

rouault Jun 30, 2024

eschnett Jul 9, 2024

eschnett commented Jul 9, 2024

rouault commented Jul 9, 2024

eschnett commented Jul 9, 2024

github-actions bot commented Aug 27, 2024

github-actions bot commented Sep 10, 2024

Initial implementation of float16 support #10342

Initial implementation of float16 support #10342

Conversation

eschnett commented Jun 30, 2024

rouault left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eschnett commented Jul 9, 2024

rouault commented Jul 9, 2024

eschnett commented Jul 9, 2024

github-actions bot commented Aug 27, 2024

github-actions bot commented Sep 10, 2024