Skip to content
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

Remove obsolete workarounds. #410

Merged
merged 8 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 4 additions & 40 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 @@ -848,19 +814,17 @@ def grid_definition_template_12(section, metadata):
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
stephenworsley marked this conversation as resolved.
Show resolved Hide resolved
# 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"])
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
70 changes: 4 additions & 66 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 @@ -578,10 +519,10 @@ def grid_definition_template_12(cube, grib):
# GRIBAPI expects unsigned ints in X1, X2, Y1, Y2 but it should accept
stephenworsley marked this conversation as resolved.
Show resolved Hide resolved
# 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 @@ -608,9 +549,6 @@ def m_to_cm(value):
# but it should accept a float, so work around this.
stephenworsley marked this conversation as resolved.
Show resolved Hide resolved
# 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.

Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,6 @@ def test__grid_points_approx(self):
self._check_key("Di", 200)
self._check_key("Dj", 500)

def test__negative_grid_points_eccodes_broken(self):
self.mock_eccodes.CodesInternalError = FakeGribError

# Force the test to run the signed int --> unsigned int workaround.
def set(grib, key, value):
if key in ["X1", "X2", "Y1", "Y2"] and value < 0:
raise self.mock_eccodes.CodesInternalError()
grib.keys[key] = value

self.mock_eccodes.codes_set = set

test_cube = self._make_test_cube(x_points=[-1, 1, 3, 5, 7], y_points=[-4, 9])
grid_definition_template_12(test_cube, self.mock_grib)
self._check_key("X1", 0x80000064)
self._check_key("X2", 700)
self._check_key("Y1", 0x80000190)
self._check_key("Y2", 900)

def test__negative_grid_points_eccodes_fixed(self):
test_cube = self._make_test_cube(x_points=[-1, 1, 3, 5, 7], y_points=[-4, 9])
grid_definition_template_12(test_cube, self.mock_grib)
Expand All @@ -138,25 +120,7 @@ def test__template_specifics(self):
self._check_key("XR", 40000000.0)
self._check_key("YR", -10000000.0)

def test__scale_factor_eccodes_broken(self):
# 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

def get_native_type(grib, key):
assert key == "scaleFactorAtReferencePoint"
return int

self.mock_eccodes.codes_get_native_type = get_native_type
grid_definition_template_12(self.test_cube, self.mock_grib)
self._check_key("scaleFactorAtReferencePoint", 1065346526)

def test__scale_factor_eccodes_fixed(self):
def get_native_type(grib, key):
assert key == "scaleFactorAtReferencePoint"
return float

self.mock_eccodes.codes_get_native_type = get_native_type
def test__scale_factor(self):
grid_definition_template_12(self.test_cube, self.mock_grib)
self._check_key("scaleFactorAtReferencePoint", 0.9996012717)

Expand Down
Loading