Skip to content

Eccodes adopt #2607

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.

Sign up for GitHub

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

Closed
marqh wants to merge 6 commits into from
Closed

Eccodes adopt #2607

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1d8bf52
adopt eccodes, use gribapi.grib_is_missing
marqh Jul 19, 2017
1597c06
review actions
marqh Jul 25, 2017
575bbad
remove == 1, test for True
marqh Jul 27, 2017
f18eb4b
f
marqh Jul 27, 2017
3a5b000
return 1
marqh Jul 27, 2017
c4d5718
use_int
marqh Jul 27, 2017
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
View file
Open in desktop
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;

@corinnebosley corinnebosley Jun 21, 2017

Copy link
Copy Markdown
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?

@marqh marqh Jun 29, 2017

Copy link
Copy Markdown
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
View file
Open in desktop
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
View file
Open in desktop
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
30 changes: 5 additions & 25 deletions lib/iris/fileformats/grib/_load_convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ def _unscale(v, f):

if isinstance(value, Iterable) or isinstance(factor, Iterable):
def _masker(item):
result = ma.masked_equal(item, _MDI)
numerical_mdi = 2 ** 32 - 1

@marqh marqh Jul 25, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

add comment here describing the logic process here, and the lack of examples or integration tests at present

item = [numerical_mdi if i is None else i for i in item]

@pelson pelson Jul 25, 2017

Copy link
Copy Markdown
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])

?

@marqh marqh Jul 27, 2017

Copy link
Copy Markdown
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
Expand All @@ -177,30 +179,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

@pelson pelson Aug 16, 2017

Copy link
Copy Markdown
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?

@pelson pelson Aug 16, 2017

Copy link
Copy Markdown
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
View file
Open in desktop
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))))

@corinnebosley corinnebosley Jun 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@marqh What is this rounding and multiplication by 1e6 for?

@marqh marqh Jun 29, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the 1e6 is a multiplier to provide values in degrees x 10 ^ -6, as per the WMO manual on codes

the round ensures proper rounding to convert to an integer

this is to ensure that the iDirectionIncrement is set correctly. this is a coded key, which are preferred by design in the iris implementation, unlike DyInDegrees which is computed. the eccodes implementation has a subtle bug in its implementation, with different behaviour between py2 and py3

we ought to follow up on this with ECMWF, but I have not yet done this

@pelson pelson Jul 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yep. Needs raising, and would also hugely benefit from a comment.

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
6 changes: 6 additions & 0 deletions lib/iris/fileformats/grib/message.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,12 @@ def _get_key_value(self, key):
# 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)
if gribapi.grib_is_missing(self._message_id, key) == 1:

@marqh marqh Jul 25, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

get_key function, with inverted order, os check, then get

add
# Implementation of Regulations 92.1.4 and 92.1.5 via eccodes.
to docstring

res = None
else:
res = gribapi.grib_get(self._message_id, key)
if gribapi.grib_is_missing(self._message_id, key) == 1:
res = None
return res

def get_computed_key(self, key):
Expand All @@ -482,6 +486,8 @@ def get_computed_key(self, key):
res = gribapi.grib_get_array(self._message_id, key)
else:
res = gribapi.grib_get(self._message_id, key)
if gribapi.grib_is_missing(self._message_id, key) == 1:
res = None
return res

def keys(self):
Expand Down
28 changes: 13 additions & 15 deletions lib/iris/tests/integration/format_interop/test_name_grib.py
View file
Open in desktop
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
View file
Open in desktop
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

@corinnebosley corinnebosley Jun 21, 2017

Copy link
Copy Markdown
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?

@marqh marqh Jun 29, 2017

Copy link
Copy Markdown
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_grid_definition_template_12.py
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_grid_definition_template_20.py
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_grid_definition_template_30.py
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_grid_definition_template_40.py
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_grid_definition_template_90.py
View file
Open in desktop
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
6 changes: 2 additions & 4 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_product_definition_template_0.py
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_product_definition_template_15.py
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions lib/iris/tests/unit/fileformats/grib/load_convert/test_product_definition_template_32.py
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion lib/iris/tests/unit/fileformats/grib/load_convert/test_unscale.py
View file
Open in desktop
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
7 changes: 6 additions & 1 deletion lib/iris/tests/unit/fileformats/grib/message/test_GribMessage.py
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions lib/iris/tests/unit/fileformats/grib/save_rules/test_grid_definition_template_0.py
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions lib/iris/tests/unit/fileformats/grib/save_rules/test_grid_definition_template_1.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,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_1(self.test_cube, self.mock_grib)
Expand Down