Skip to content

Commit

Permalink
Remove obsolete workarounds. (SciTools#410)
Browse files Browse the repository at this point in the history
* Remove obsolete workarounds.

* Remove dead tests.

* Remove more dead tests.

* Remove now-unused workaround routines.

* Review changes.

* Review change: remove another obsolete comment

---------

Co-authored-by: stephenworsley <[email protected]>
  • Loading branch information
pp-mo and stephenworsley authored Apr 19, 2024
1 parent a9d45b1 commit 7eacda5
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 328 deletions.
49 changes: 5 additions & 44 deletions iris_grib/_load_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,40 +225,6 @@ def _hindcast_fix(forecast_time):
return forecast_time


def fixup_float32_from_int32(value):
"""
Workaround for use when reading an IEEE 32-bit floating-point value
which the ECMWF GRIB API has erroneously treated as a 4-byte signed
integer.
"""
# Convert from two's complement to sign-and-magnitude.
# NB. The bit patterns 0x00000000 and 0x80000000 will both be
# returned by the ECMWF GRIB API as an integer 0. Because they
# correspond to positive and negative zero respectively it is safe
# to treat an integer 0 as a positive zero.
if value < 0:
value = 0x80000000 - value
value_as_uint32 = np.array(value, dtype="u4")
value_as_float32 = value_as_uint32.view(dtype="f4")
return float(value_as_float32)


def fixup_int32_from_uint32(value):
"""
Workaround for use when reading a signed, 4-byte integer which the
ECMWF GRIB API has erroneously treated as an unsigned, 4-byte
integer.
NB. This workaround is safe to use with values which are already
treated as signed, 4-byte integers.
"""
if value >= 0x80000000:
value = 0x80000000 - value
return value


###############################################################################
#
# Identification Section 1
Expand Down Expand Up @@ -846,21 +812,16 @@ def grid_definition_template_12(section, metadata):
lat = section["latitudeOfReferencePoint"] * _GRID_ACCURACY_IN_DEGREES
lon = section["longitudeOfReferencePoint"] * _GRID_ACCURACY_IN_DEGREES
scale = section["scaleFactorAtReferencePoint"]
# Catch bug in ECMWF GRIB API (present at 1.12.1) where the scale
# is treated as a signed, 4-byte integer.
if isinstance(scale, int):
scale = fixup_float32_from_int32(scale)
CM_TO_M = 0.01
easting = section["XR"] * CM_TO_M
northing = section["YR"] * CM_TO_M
cs = icoord_systems.TransverseMercator(lat, lon, easting, northing, scale, geog_cs)

# Deal with bug in ECMWF GRIB API (present at 1.12.1) where these
# values are treated as unsigned, 4-byte integers.
x1 = fixup_int32_from_uint32(section["X1"])
y1 = fixup_int32_from_uint32(section["Y1"])
x2 = fixup_int32_from_uint32(section["X2"])
y2 = fixup_int32_from_uint32(section["Y2"])
# Fetch grid extents
x1 = section["X1"]
y1 = section["Y1"]
x2 = section["X2"]
y2 = section["Y2"]

# Rather unhelpfully this grid definition template seems to be
# overspecified, and thus open to inconsistency. But for determining
Expand Down
78 changes: 4 additions & 74 deletions iris_grib/_save_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,65 +48,6 @@
_TIME_RANGE_UNITS_INVERTED = {val: key for key, val in _TIME_RANGE_UNITS.items()}


def fixup_float32_as_int32(value):
"""
Workaround for use when the ECMWF GRIB API treats an IEEE 32-bit
floating-point value as a signed, 4-byte integer.
Returns the integer value which will result in the on-disk
representation corresponding to the IEEE 32-bit floating-point
value.
"""
value_as_float32 = np.array(value, dtype="f4")
value_as_uint32 = value_as_float32.view(dtype="u4")
if value_as_uint32 >= 0x80000000:
# Convert from two's-complement to sign-and-magnitude.
# NB. Because of the silly representation of negative
# integers in GRIB2, there is no value we can pass to
# codes_set that will result in the bit pattern 0x80000000.
# But since that bit pattern corresponds to a floating
# point value of negative-zero, we can safely treat it as
# positive-zero instead.
value_as_grib_int = 0x80000000 - int(value_as_uint32)
else:
value_as_grib_int = int(value_as_uint32)
return value_as_grib_int


def fixup_int32_as_uint32(value):
"""
Workaround for use when the ECMWF GRIB API treats a signed, 4-byte
integer value as an unsigned, 4-byte integer.
Returns the unsigned integer value which will result in the on-disk
representation corresponding to the signed, 4-byte integer value.
"""
value = int(value)
if -0x7FFFFFFF <= value <= 0x7FFFFFFF:
if value < 0:
# Convert from two's-complement to sign-and-magnitude.
value = 0x80000000 - value
else:
msg = "{} out of range -2147483647 to 2147483647.".format(value)
raise ValueError(msg)
return value


def ensure_set_int32_value(grib, key, value):
"""
Ensure the workaround function :func:`fixup_int32_as_uint32` is applied as
necessary to problem keys.
"""
try:
eccodes.codes_set(grib, key, value)
except eccodes.CodesInternalError:
value = fixup_int32_as_uint32(value)
eccodes.codes_set(grib, key, value)


###############################################################################
#
# Constants
Expand Down Expand Up @@ -574,14 +515,10 @@ def grid_definition_template_12(cube, grib):
eccodes.codes_set(grib, "Di", abs(x_step))
eccodes.codes_set(grib, "Dj", abs(y_step))
horizontal_grid_common(cube, grib)

# GRIBAPI expects unsigned ints in X1, X2, Y1, Y2 but it should accept
# signed ints, so work around this.
# See https://software.ecmwf.int/issues/browse/SUP-1101
ensure_set_int32_value(grib, "Y1", int(y_cm[0]))
ensure_set_int32_value(grib, "Y2", int(y_cm[-1]))
ensure_set_int32_value(grib, "X1", int(x_cm[0]))
ensure_set_int32_value(grib, "X2", int(x_cm[-1]))
eccodes.codes_set(grib, "Y1", int(y_cm[0]))
eccodes.codes_set(grib, "Y2", int(y_cm[-1]))
eccodes.codes_set(grib, "X1", int(x_cm[0]))
eccodes.codes_set(grib, "X2", int(x_cm[-1]))

# Lat and lon of reference point are measured in millionths of a degree.
eccodes.codes_set(
Expand All @@ -603,14 +540,7 @@ def m_to_cm(value):
# False easting and false northing are measured in units of (10^-2)m.
eccodes.codes_set(grib, "XR", m_to_cm(cs.false_easting))
eccodes.codes_set(grib, "YR", m_to_cm(cs.false_northing))

# GRIBAPI expects a signed int for scaleFactorAtReferencePoint
# but it should accept a float, so work around this.
# See https://software.ecmwf.int/issues/browse/SUP-1100
value = cs.scale_factor_at_central_meridian
key_type = eccodes.codes_get_native_type(grib, "scaleFactorAtReferencePoint")
if key_type is not float:
value = fixup_float32_as_int32(value)
eccodes.codes_set(grib, "scaleFactorAtReferencePoint", value)


Expand Down
32 changes: 0 additions & 32 deletions iris_grib/tests/unit/load_convert/test_fixup_float32_from_int32.py

This file was deleted.

42 changes: 0 additions & 42 deletions iris_grib/tests/unit/load_convert/test_fixup_int32_from_uint32.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -184,17 +184,6 @@ def test_incompatible_grid_extent(self):
with self.assertRaisesRegex(iris.exceptions.TranslationError, "grid"):
grid_definition_template_12(section, metadata)

def test_scale_workaround(self):
section = self.section_3()
section["scaleFactorAtReferencePoint"] = 1065346526
metadata = empty_metadata()
grid_definition_template_12(section, metadata)
expected = self.expected(0, 1)
# A float32 can't hold exactly the same value.
cs = expected["dim_coords_and_dims"][0][0].coord_system
cs.scale_factor_at_central_meridian = 0.9996012449264526
self.assertEqual(metadata, expected)


if __name__ == "__main__":
tests.main()
48 changes: 0 additions & 48 deletions iris_grib/tests/unit/save_rules/test_fixup_float32_as_int32.py

This file was deleted.

40 changes: 0 additions & 40 deletions iris_grib/tests/unit/save_rules/test_fixup_int32_as_uint32.py

This file was deleted.

Loading

0 comments on commit 7eacda5

Please sign in to comment.