#6879 closed defect (fixed)
Behavior changes with GDALDataTypeUnion from r38182
| Reported by: | Kurt Schwehr | Owned by: | warmerdam |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | default | Version: | unspecified |
| Severity: | normal | Keywords: | |
| Cc: |
Description
On applying r38182, I see changes with GdalMiscTest.GdalDataTypeUnion in https://github.com/schwehr/gdal-autotest2/blob/master/cpp/gcore/gdal_misc_test.cc
Where these behavior changes intended by this?
https://github.com/OSGeo/gdal/pull/215/files#diff-d71cafa6afd78f5a5b978d8920b85633
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:40
Expected equality of these values:
GDT_Int16
Which is: 3
GDALDataTypeUnion(GDT_Int16, GDT_UInt16)
Which is: 5
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:41
Expected equality of these values:
GDT_Int32
Which is: 5
GDALDataTypeUnion(GDT_Int16, GDT_UInt32)
Which is: 7
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:42
Expected equality of these values:
GDT_Int32
Which is: 5
GDALDataTypeUnion(GDT_UInt32, GDT_Int16)
Which is: 7
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:43
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_UInt32, GDT_CInt16)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:46
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_Float32, GDT_CInt32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:48
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt16, GDT_UInt32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:50
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CInt16, GDT_CFloat32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:52
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt32, GDT_Byte)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:53
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt32, GDT_UInt16)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:54
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt32, GDT_Int16)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:55
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt32, GDT_UInt32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:56
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt32, GDT_Int32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:57
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CInt32, GDT_Float32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:58
Expected equality of these values:
GDT_CInt32
Which is: 9
GDALDataTypeUnion(GDT_CInt32, GDT_CInt16)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:59
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CInt32, GDT_CFloat32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:61
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_Byte)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:62
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_UInt16)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:63
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_Int16)
Which is: 11
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_UInt32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:65
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_Int32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:66
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_Float32)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:68
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_CInt16)
Which is: 11
third_party/gdal/autotest2/cpp/gcore/gdal_misc_test.cc:69
Expected equality of these values:
GDT_CFloat32
Which is: 10
GDALDataTypeUnion(GDT_CFloat32, GDT_CInt32)
Which is: 11
Change History (14)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
@kurt Such tests would be more than welcome in the upstream testing infrastructure...
comment:3 by , 7 years ago
I presume that the community isn't interested in having both tut and gunit based tests in the tree. I'd be happy if anyone wants to port any of the autotest2 C++ tests to tut.
comment:5 by , 7 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
I think we are still in the land of some regressions (bummer) and some fixes (woohoo!).
Discussion of why those changes are the way they are w.r.t. IEEE 754 would be good. e.g. https://en.wikipedia.org/wiki/IEEE_floating_point#Basic_and_interchange_formats would be good.
Here is the overall diff I had to apply to get my autotest2 gdal_misc_test.cc to pass:
-
.cc
diff -u gdal_misc_test.cc gdal_misc_test_new.cc
old new 36 36 EXPECT_EQ(GDT_Unknown, GDALDataTypeUnion(GDT_Unknown, GDT_CFloat64)); 37 37 EXPECT_EQ(GDT_UInt16, GDALDataTypeUnion(GDT_Byte, GDT_UInt16)); 38 38 EXPECT_EQ(GDT_Int16, GDALDataTypeUnion(GDT_Byte, GDT_Int16)); 39 EXPECT_EQ(GDT_Int 16, GDALDataTypeUnion(GDT_Int16, GDT_UInt16));40 EXPECT_EQ(GDT_ Int32, GDALDataTypeUnion(GDT_Int16, GDT_UInt32));41 EXPECT_EQ(GDT_ Int32, GDALDataTypeUnion(GDT_UInt32, GDT_Int16));42 EXPECT_EQ(GDT_C Int32, GDALDataTypeUnion(GDT_UInt32, GDT_CInt16));39 EXPECT_EQ(GDT_Int32, GDALDataTypeUnion(GDT_Int16, GDT_UInt16)); 40 EXPECT_EQ(GDT_Float64, GDALDataTypeUnion(GDT_Int16, GDT_UInt32)); 41 EXPECT_EQ(GDT_Float64, GDALDataTypeUnion(GDT_UInt32, GDT_Int16)); 42 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_UInt32, GDT_CInt16)); 43 43 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_Int32, GDT_CInt16)); 44 44 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_Float32, GDT_CInt16)); 45 EXPECT_EQ(GDT_CFloat 32, GDALDataTypeUnion(GDT_Float32, GDT_CInt32));45 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_Float32, GDT_CInt32)); 46 46 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_Float64, GDT_CInt32)); 47 EXPECT_EQ(GDT_C Int32, GDALDataTypeUnion(GDT_CInt16, GDT_UInt32));47 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt16, GDT_UInt32)); 48 48 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt16, GDT_Int32)); 49 49 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CInt16, GDT_CFloat32)); 50 50 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt16, GDT_CFloat64)); 51 51 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt32, GDT_Byte)); 52 52 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt32, GDT_UInt16)); 53 53 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt32, GDT_Int16)); 54 EXPECT_EQ(GDT_C Int32, GDALDataTypeUnion(GDT_CInt32, GDT_UInt32));54 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt32, GDT_UInt32)); 55 55 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt32, GDT_Int32)); 56 EXPECT_EQ(GDT_CFloat 32, GDALDataTypeUnion(GDT_CInt32, GDT_Float32));56 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt32, GDT_Float32)); 57 57 EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt32, GDT_CInt16)); 58 EXPECT_EQ(GDT_CFloat 32, GDALDataTypeUnion(GDT_CInt32, GDT_CFloat32));58 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt32, GDT_CFloat32)); 59 59 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt32, GDT_CFloat64)); 60 60 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_Byte)); 61 61 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_UInt16)); 62 62 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_Int16)); 63 EXPECT_EQ(GDT_CFloat 32, GDALDataTypeUnion(GDT_CFloat32, GDT_UInt32));64 EXPECT_EQ(GDT_CFloat 32, GDALDataTypeUnion(GDT_CFloat32, GDT_Int32));63 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat32, GDT_UInt32)); 64 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat32, GDT_Int32)); 65 65 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_Float32)); 66 66 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat32, GDT_Float64)); 67 67 EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_CInt16)); 68 EXPECT_EQ(GDT_CFloat 32, GDALDataTypeUnion(GDT_CFloat32, GDT_CInt32));68 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat32, GDT_CInt32)); 69 69 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat64, GDT_Int32)); 70 70 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat64, GDT_Float32)); 71 71 EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat64, GDT_Float64));
The things that I see that are not well packed:
Why are these now floats? I think these are incorrect changes
- EXPECT_EQ(GDT_Int32, GDALDataTypeUnion(GDT_Int16, GDT_UInt32)); + EXPECT_EQ(GDT_Float64, GDALDataTypeUnion(GDT_Int16, GDT_UInt32));
- EXPECT_EQ(GDT_Int32, GDALDataTypeUnion(GDT_UInt32, GDT_Int16)); + EXPECT_EQ(GDT_Float64, GDALDataTypeUnion(GDT_UInt32, GDT_Int16));
- EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_UInt32, GDT_CInt16)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_UInt32, GDT_CInt16));
GDT_CFloat32 too small to handle int32, so I think this is okay.
- EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_Float32, GDT_CInt32)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_Float32, GDT_CInt32));
Neither is correct. I think these should be GDT_CInt64. Why float?
- EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt16, GDT_UInt32)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt16, GDT_UInt32));
- EXPECT_EQ(GDT_CInt32, GDALDataTypeUnion(GDT_CInt32, GDT_UInt32)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt32, GDT_UInt32));
Ok? We need 64bit float with it's 253 mantissa space as float32 only has 224 for the mantisa.
- EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CInt32, GDT_Float32)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CInt32, GDT_Float32));
- EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_UInt32)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat32, GDT_UInt32));
- EXPECT_EQ(GDT_CFloat32, GDALDataTypeUnion(GDT_CFloat32, GDT_Int32)); + EXPECT_EQ(GDT_CFloat64, GDALDataTypeUnion(GDT_CFloat32, GDT_Int32));
comment:6 by , 7 years ago
GDT_Int32 is not large enough to hold GDT_Int16, GDT_UInt32. If we had GDT_Int64 it would be the best fit, but in the absence of it GDT_Float64 seems the more appropriate.
Same for GDT_CInt64.
So it seems to me that the new results are OK
follow-up: 8 comment:7 by , 7 years ago
Errr... sorry. I didn't realize we don't have a GDT_Int64 or GDT_CInt64. If a platform has the capability, should be have that pixel type available? If not, a comment as to why would be helpful.
comment:8 by , 7 years ago
Errr... sorry. I didn't realize we don't have a GDT_Int64 or GDT_CInt64. If a platform has the capability, should be have that pixel type available?
That could be likely worth a RFC (since it impacts API behaviour), and an update in some drivers like netCDF to use it
follow-up: 10 comment:9 by , 7 years ago
Can someone who knows why, add a comment as to why there it doesn't have GDT_Int64, GDT_CInt64, GDT_UInt64?
comment:10 by , 7 years ago
Replying to Kurt Schwehr:
Can someone who knows why, add a comment as to why there it doesn't have GDT_Int64, GDT_CInt64, GDT_UInt64?
I'm not sure in which place exactly you'd want that comment: in the tests ? And is your question "why isn't there GDT_Int64, GDT_CInt64, GDT_UInt64?" If so, I guess the answer is "because nobody care enough yet to add support for them".
comment:11 by , 7 years ago
I was thinking in the enum...
https://trac.osgeo.org/gdal/browser/trunk/gdal/gcore/gdal.h?rev=38182#L70
e.g. maybe something like this or take your pick as long as it's somewhat easy to figure out that those types are known to not be there.
/*! Pixel data types */
typedef enum {
/*! Unknown or unspecified type */ GDT_Unknown = 0,
/*! Eight bit unsigned integer */ GDT_Byte = 1,
/*! Sixteen bit unsigned integer */ GDT_UInt16 = 2,
/*! Sixteen bit signed integer */ GDT_Int16 = 3,
/*! Thirty two bit unsigned integer */ GDT_UInt32 = 4,
/*! Thirty two bit signed integer */ GDT_Int32 = 5,
/* TODO(#6879): GDT_Int64 */
/* TODO(#6879): GDT_UInt64 */
/*! Thirty two bit floating point */ GDT_Float32 = 6,
/*! Sixty four bit floating point */ GDT_Float64 = 7,
/*! Complex Int16 */ GDT_CInt16 = 8,
/*! Complex Int32 */ GDT_CInt32 = 9,
/* TODO(#6879): GDT_CInt64 */
/*! Complex Float32 */ GDT_CFloat32 = 10,
/*! Complex Float64 */ GDT_CFloat64 = 11,
GDT_TypeCount = 12 /* maximum type # + 1 */
} GDALDataType;
comment:13 by , 7 years ago
This week, I hit my first HDF5 64-bit integer value with a JAXA file. There isn't a rush to add GDT_Int64 and GDT_UInt64 as it's possible to load the data from python with numpy and only the low order bits are set.
gdalinfo -mm -stats -listmdd GPMMRG_MAP_0007021800_H_L3S_MCH_03E.h5 Subdatasets: SUBDATASET_1_NAME=HDF5:"GPMMRG_MAP_0007021800_H_L3S_MCH_03E.h5"://Grid/gaugeQualityInfo SUBDATASET_1_DESC=[3600x1800] //Grid/gaugeQualityInfo (16-bit integer) SUBDATASET_2_NAME=HDF5:"GPMMRG_MAP_0007021800_H_L3S_MCH_03E.h5"://Grid/hourlyPrecipRate SUBDATASET_2_DESC=[3600x1800] //Grid/hourlyPrecipRate (32-bit floating-point) SUBDATASET_3_NAME=HDF5:"GPMMRG_MAP_0007021800_H_L3S_MCH_03E.h5"://Grid/hourlyPrecipRateGC SUBDATASET_3_DESC=[3600x1800] //Grid/hourlyPrecipRateGC (32-bit floating-point) SUBDATASET_4_NAME=HDF5:"GPMMRG_MAP_0007021800_H_L3S_MCH_03E.h5"://Grid/observationTimeFlag SUBDATASET_4_DESC=[3600x1800] //Grid/observationTimeFlag (32-bit floating-point)
satelliteInfoFlag is missing. The tail end of h5dump -H:
DATASET "satelliteInfoFlag" {
DATATYPE H5T_STD_I64LE
DATASPACE SIMPLE { ( 3600, 1800 ) / ( 3600, 1800 ) }
ATTRIBUTE "CodeMissingValue" {
DATATYPE H5T_STRING {
STRSIZE 6;
STRPAD H5T_STR_NULLTERM;
CSET H5T_CSET_ASCII;
CTYPE H5T_C_S1;
}
DATASPACE SCALAR
}
ATTRIBUTE "DimensionNames" {
DATATYPE H5T_STRING {
STRSIZE 10;
STRPAD H5T_STR_NULLTERM;
CSET H5T_CSET_ASCII;
CTYPE H5T_C_S1;
}
DATASPACE SCALAR
}
ATTRIBUTE "_FillValue" {
DATATYPE H5T_STD_I64LE
DATASPACE SCALAR
}
}
From somewhere in https://www.gportal.jaxa.jp/gp/top.html
comment:14 by , 7 years ago
satelliteInfoFlag is defined here:
https://storm.pps.eosdis.nasa.gov/storm/data/docs/filespec.GPM.V1.3GSMAPH.pdf
GDALDataTypeUnion(GDT_Int16, GDT_UInt16) now returning GDT_Int32 seems to be an improvement (since GDT_Int16 cannot hold the full range of uint16)
But GDALDataTypeUnion(GDT_CInt32, GDT_Byte) now returning GDT_CFloat64 instead of GDT_CInt32 seems to be a regression.
Note: I didn't check everything in the above log