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

Eccodes adopt #2607

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ install:
conda install --quiet --file minimal-conda-requirements.txt;
else
if [[ "$TRAVIS_PYTHON_VERSION" == 3* ]]; then
sed -e '/ecmwf_grib/d' -e '/esmpy/d' -e 's/#.\+$//' conda-requirements.txt | xargs conda install --quiet;
sed -e '/esmpy/d' -e 's/#.\+$//' conda-requirements.txt | xargs conda install --quiet;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marqh @bjlittle @pp-mo Can anybody explain to me what difference this makes please?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nasty hack to make sure ecmwf_grib is not installed for python3, where it won't build. This is no longer required as eccodes is py3 compatible

else
conda install --quiet --file conda-requirements.txt;
fi
Expand Down
5 changes: 3 additions & 2 deletions INSTALL
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ gdal 1.9.1 or later (https://pypi.python.org/pypi/GDAL/)
graphviz 2.18 or later (http://www.graphviz.org/)
Graph visualisation software.

grib-api 1.9.16 or later
(https://software.ecmwf.int/wiki/display/GRIB/Releases)
eccodes
(https://software.ecmwf.int/wiki/display/ECC/ecCodes+Home)
API for the encoding and decoding WMO FM-92 GRIB edition 1 and
edition 2 messages. A compression library such as Jasper is required
to read JPEG2000 compressed GRIB2 files.
Successor to GRIBAPI.

matplotlib 1.2.0 (http://matplotlib.sourceforge.net/)
Python package for 2D plotting.
Expand Down
2 changes: 1 addition & 1 deletion conda-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ imagehash
requests

# Optional iris dependencies
python-ecmwf_grib
python-eccodes
esmpy>=7.0
gdal
libmo_unpack
Expand Down
40 changes: 14 additions & 26 deletions lib/iris/fileformats/grib/_load_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,21 @@ def _unscale(v, f):

if isinstance(value, Iterable) or isinstance(factor, Iterable):
def _masker(item):
result = ma.masked_equal(item, _MDI)
# This is a small work around for an edge case, which is not
# evident in any of our sample GRIB2 messages, where an array
# of header elements contains missing values.
# iris.fileformats.grib.message returns these as None, but they
# are wanted as a numerical masked array, so a temporary mdi
# value is used, selected from a legacy implementation of iris,
# to construct the masked array. The valure is transient, only in
# scope for this function.
numerical_mdi = 2 ** 32 - 1
item = [numerical_mdi if i is None else i for i in item]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be better done as:

np.ma.masked_array(item, mask=[i is None for i in item])

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe not. If this were implemented, then a mixed type underlying array would be created, with floats and None, so NumPy would have to make an object array. This would be a behaviour change, which I don't think is desired.

result = ma.masked_equal(item, numerical_mdi)
if ma.count_masked(result):
# Circumvent downstream NumPy "RuntimeWarning"
# of "overflow encountered in power" in _unscale
# for data containing _MDI.
# for data containing _MDI. Remove transient _MDI value.
result.data[result.mask] = 0
return result
value = _masker(value)
Expand All @@ -177,30 +187,8 @@ def _masker(item):
return result


# Regulations 92.1.4 and 92.1.5.
_MDI = 2 ** 32 - 1
# Note:
# 1. Integer "on-disk" values (aka. coded keys) in GRIB messages:
# - Are 8-, 16-, or 32-bit.
# - Are either signed or unsigned, with signed values stored as
# sign-and-magnitude (*not* twos-complement).
# - Use all bits set to indicate a missing value (MDI).
# 2. Irrespective of the on-disk form, the ECMWF GRIB API *always*:
# - Returns values as 64-bit signed integers, either as native
# Python 'int' or numpy 'int64'.
# - Returns missing values as 2**32 - 1, but not all keys are
# defined as supporting missing values.
# NB. For keys which support missing values, the MDI value is reliably
# distinct from the valid range of either signed or unsigned 8-, 16-,
# or 32-bit values. For example:
# unsigned 32-bit:
# min = 0b000...000 = 0
# max = 0b111...110 = 2**32 - 2
# MDI = 0b111...111 = 2**32 - 1
# signed 32-bit:
# MDI = 0b111...111 = 2**32 - 1
# min = 0b111...110 = -(2**31 - 2)
# max = 0b011...111 = 2**31 - 1
# Use ECCodes gribapi to recognise missing value
_MDI = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we now remove this altogether?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the answer is - some of the tests assert that a thing is missing, and this is used for that purpose.
The comment for this line doesn't really help in that regard. Would you mind adding something?



# Non-standardised usage for negative forecast times.
Expand Down
17 changes: 15 additions & 2 deletions lib/iris/fileformats/grib/_save_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,21 @@ def latlon_first_last(x_coord, y_coord, grib):
def dx_dy(x_coord, y_coord, grib):
x_step = regular_step(x_coord)
y_step = regular_step(y_coord)
gribapi.grib_set(grib, "DxInDegrees", float(abs(x_step)))
gribapi.grib_set(grib, "DyInDegrees", float(abs(y_step)))
# Set x and y step. For degrees, this is encoded as an integer:
# 1 * 10^6 * floating point value.
# WMO Manual on Codes regulation 92.1.6
if x_coord.units == 'degrees':
gribapi.grib_set(grib, "iDirectionIncrement",
round(1e6 * float(abs(x_step))))
else:
raise ValueError('X coordinate must be in degrees, not {}'
'.'.format(x_coord.units))
if y_coord.units == 'degrees':
gribapi.grib_set(grib, "jDirectionIncrement",
round(1e6 * float(abs(y_step))))
else:
raise ValueError('Y coordinate must be in degrees, not {}'
'.'.format(y_coord.units))


def scanning_mode_flags(x_coord, y_coord, grib):
Expand Down
21 changes: 18 additions & 3 deletions lib/iris/fileformats/grib/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,9 @@ def _get_key_value(self, key):
elif key in ('typeOfFirstFixedSurface', 'typeOfSecondFixedSurface'):
# By default these values are returned as unhelpful strings but
# we can use int representation to compare against instead.
res = gribapi.grib_get(self._message_id, key, int)
res = self._get_value_or_missing(key, use_int=True)
else:
res = gribapi.grib_get(self._message_id, key)
res = self._get_value_or_missing(key)
return res

def get_computed_key(self, key):
Expand All @@ -481,9 +481,24 @@ def get_computed_key(self, key):
if key in vector_keys:
res = gribapi.grib_get_array(self._message_id, key)
else:
res = gribapi.grib_get(self._message_id, key)
res = self._get_value_or_missing(key)
return res

def keys(self):
"""Return coded keys available in this Section."""
return self._keys

def _get_value_or_missing(self, key, use_int=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for having factored this out
Rather than a binary toggle, would you mind passing the dtype of the expected result?

"""
Return value of header element, or None if value is encoded as missing.
Implementation of Regulations 92.1.4 and 92.1.5 via ECCodes.

"""
if gribapi.grib_is_missing(self._message_id, key):
result = None
else:
if use_int:
result = gribapi.grib_get(self._message_id, key, int)
else:
result = gribapi.grib_get(self._message_id, key)
return result
28 changes: 13 additions & 15 deletions lib/iris/tests/integration/format_interop/test_name_grib.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# (C) British Crown Copyright 2013 - 2016, Met Office
# (C) British Crown Copyright 2013 - 2017, Met Office
#
# This file is part of Iris.
#
Expand Down Expand Up @@ -72,21 +72,19 @@ def check_common(self, name_cube, grib_cube):
def test_name2_field(self):
filepath = tests.get_data_path(('NAME', 'NAMEII_field.txt'))
name_cubes = iris.load(filepath)
# Check gribapi version, because we currently have a known load/save
# problem with gribapi 1v14 (at least).
gribapi_ver = gribapi.grib_get_api_version()
gribapi_fully_supported_version = \
(StrictVersion(gribapi.grib_get_api_version()) <
StrictVersion('1.13'))

# There is a known load/save problem with numerous
# gribapi/eccodes versions and
# zero only data, where min == max.
# This may be a problem with data scaling.
for i, name_cube in enumerate(name_cubes):
if not gribapi_fully_supported_version:
data = name_cube.data
if np.min(data) == np.max(data):
msg = ('NAMEII cube #{}, "{}" has empty data : '
'SKIPPING test for this cube, as save/load will '
'not currently work with gribabi > 1v12.')
warnings.warn(msg.format(i, name_cube.name()))
continue
data = name_cube.data
if np.min(data) == np.max(data):
msg = ('NAMEII cube #{}, "{}" has empty data : '
'SKIPPING test for this cube, as save/load will '
'not currently work.')
warnings.warn(msg.format(i, name_cube.name()))
continue

with self.temp_filename('.grib2') as temp_filename:
iris.save(name_cube, temp_filename)
Expand Down
15 changes: 8 additions & 7 deletions lib/iris/tests/test_grib_save.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# (C) British Crown Copyright 2010 - 2016, Met Office
# (C) British Crown Copyright 2010 - 2017, Met Office
#
# This file is part of Iris.
#
Expand Down Expand Up @@ -34,6 +34,7 @@

if tests.GRIB_AVAILABLE:
import gribapi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marqh I don't understand why this import is still here. Is this not being replaced?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ECCodes provides a module called gribapi, it is a replacement for GRIB_API

see
https://software.ecmwf.int/wiki/display/ECC/GRIB-API+migration
for more information

thus, the import name is unchanged

from iris.fileformats.grib._load_convert import _MDI as MDI


@tests.skip_data
Expand All @@ -50,10 +51,10 @@ def test_latlon_forecast_plev(self):
iris.save(cubes, temp_file_path)
expect_diffs = {'totalLength': (4837, 4832),
'productionStatusOfProcessedData': (0, 255),
'scaleFactorOfRadiusOfSphericalEarth': (4294967295,
'scaleFactorOfRadiusOfSphericalEarth': (MDI,
0),
'shapeOfTheEarth': (0, 1),
'scaledValueOfRadiusOfSphericalEarth': (4294967295,
'scaledValueOfRadiusOfSphericalEarth': (MDI,
6367470),
'typeOfGeneratingProcess': (0, 255),
'generatingProcessIdentifier': (128, 255),
Expand All @@ -70,10 +71,10 @@ def test_rotated_latlon(self):
iris.save(cubes, temp_file_path)
expect_diffs = {'totalLength': (648196, 648191),
'productionStatusOfProcessedData': (0, 255),
'scaleFactorOfRadiusOfSphericalEarth': (4294967295,
'scaleFactorOfRadiusOfSphericalEarth': (MDI,
0),
'shapeOfTheEarth': (0, 1),
'scaledValueOfRadiusOfSphericalEarth': (4294967295,
'scaledValueOfRadiusOfSphericalEarth': (MDI,
6367470),
'longitudeOfLastGridPoint': (392109982, 32106370),
'latitudeOfLastGridPoint': (19419996, 19419285),
Expand All @@ -91,10 +92,10 @@ def test_time_mean(self):
cubes = iris.load(source_grib)
expect_diffs = {'totalLength': (21232, 21227),
'productionStatusOfProcessedData': (0, 255),
'scaleFactorOfRadiusOfSphericalEarth': (4294967295,
'scaleFactorOfRadiusOfSphericalEarth': (MDI,
0),
'shapeOfTheEarth': (0, 1),
'scaledValueOfRadiusOfSphericalEarth': (4294967295,
'scaledValueOfRadiusOfSphericalEarth': (MDI,
6367470),
'longitudeOfLastGridPoint': (356249908, 356249809),
'latitudeOfLastGridPoint': (-89999938, -89999944),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,10 @@
import iris.coords
import iris.exceptions
from iris.fileformats.grib._load_convert import grid_definition_template_12
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests.unit.fileformats.grib.load_convert import empty_metadata


MDI = 2 ** 32 - 1


class Test(tests.IrisTest):
def section_3(self):
section = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@
import iris.coord_systems
import iris.coords
from iris.fileformats.grib._load_convert import grid_definition_template_20
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests.unit.fileformats.grib.load_convert import empty_metadata


MDI = 2 ** 32 - 1


class Test(tests.IrisTest):

def section_3(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@
import iris.coord_systems
import iris.coords
from iris.fileformats.grib._load_convert import grid_definition_template_30
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests.unit.fileformats.grib.load_convert import empty_metadata


MDI = 2 ** 32 - 1


class Test(tests.IrisTest):

def section_3(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@
import iris.coord_systems
import iris.coords
from iris.fileformats.grib._load_convert import grid_definition_template_40
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests.unit.fileformats.grib.load_convert import empty_metadata


MDI = 2 ** 32 - 1


class _Section(dict):
def get_computed_key(self, key):
return self.get(key)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@
import iris.coords
import iris.exceptions
from iris.fileformats.grib._load_convert import grid_definition_template_90
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests.unit.fileformats.grib.load_convert import empty_metadata


MDI = 2 ** 32 - 1


class Test(tests.IrisTest):
def uk(self):
section = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# (C) British Crown Copyright 2014 - 2015, Met Office
# (C) British Crown Copyright 2014 - 2017, Met Office
#
# This file is part of Iris.
#
Expand Down Expand Up @@ -30,13 +30,11 @@
import iris.coords
from iris.tests.unit.fileformats.grib.load_convert import (LoadConvertTest,
empty_metadata)
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.fileformats.grib._load_convert import product_definition_template_0
from iris.tests import mock


MDI = 0xffffffff


def section_4():
return {'hoursAfterDataCutoff': MDI,
'minutesAfterDataCutoff': MDI,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,11 @@
from iris.coords import CellMethod, DimCoord
from iris.exceptions import TranslationError
from iris.fileformats.grib._load_convert import product_definition_template_15
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests.unit.fileformats.grib.load_convert import (LoadConvertTest,
empty_metadata)


MDI = 0xffffffff


def section_4():
return {'productDefinitionTemplateNumber': 15,
'hoursAfterDataCutoff': MDI,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@
import iris.tests as tests

from iris.fileformats.grib._load_convert import product_definition_template_32
from iris.fileformats.grib._load_convert import _MDI as MDI
from iris.tests import mock
from iris.tests.unit.fileformats.grib.load_convert import empty_metadata


MDI = 0xffffffff


class Test(tests.IrisTest):
def setUp(self):
self.patch('warnings.warn')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def test_array(self):
def test_array_mdi(self):
result = unscale([1, MDI, 100, 1000], [1, 1, 1, MDI])
self.assertTrue(ma.isMaskedArray(result))
expected = ma.masked_values([0.1, MDI, 10.0, MDI], MDI)
expected = ma.masked_values([0.1, 0, 10.0, 0], 0)
np.testing.assert_array_almost_equal(result, expected)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ def test_release_file(self):
filename = tests.get_data_path(('GRIB', '3_layer_viz',
'3_layer.grib2'))
my_file = open(filename)
self.patch('__builtin__.open', mock.Mock(return_value=my_file))
if six.PY2:
self.patch('__builtin__.open', mock.Mock(return_value=my_file))
else:
import builtins
self.patch('builtins.open', mock.Mock(return_value=my_file))

messages = list(GribMessage.messages_from_filename(filename))
self.assertFalse(my_file.closed)
del messages
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ def test__grid_points(self):
self._check_key("longitudeOfLastGridPoint", 7000000)
self._check_key("latitudeOfFirstGridPoint", 4000000)
self._check_key("latitudeOfLastGridPoint", 9000000)
self._check_key("DxInDegrees", 2.0)
self._check_key("DyInDegrees", 5.0)
self._check_key("iDirectionIncrement", 2000000)
self._check_key("jDirectionIncrement", 5000000)

def test__scanmode(self):
grid_definition_template_0(self.test_cube, self.mock_grib)
Expand Down
Loading