Skip to content

Fix aux-factory lazy bounds. #10

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

Merged
merged 2 commits into from
Jun 7, 2017
Merged

Fix aux-factory lazy bounds. #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
949aecd
Fix aux-factory lazy bounds.
bjlittle Jun 5, 2017
07ee9a0
Review actions.
bjlittle Jun 7, 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
1 change: 0 additions & 1 deletion lib/iris/aux_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ def _remap_with_bounds(self, dependency_dims, derived_dims):
# extra dimension to make the shape compatible, so
# we just add an extra 1.
shape.append(1)
nd_values = np.array(nd_values)
Copy link
Author

Choose a reason for hiding this comment

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

@corinnebosley Maybe this isn't a safe thing to do ... dunno. Most likely, not.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm still not sure about this. It makes me very nervous just whipping out a safety check (wow, how Sheldon is that??).

Although since you have checked through the code path to make sure that there is no way for a non-array (i.e. a value) to appear here, I am a little happier about it.

My trigger finger is itchy, but I also have ants in my pants about this a bit. Not sure what to do...

Copy link
Author

@bjlittle bjlittle Jun 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

We can use test coverage to convince ourselves that his is a sensible change ... Ba-zinga!

Copy link
Author

Choose a reason for hiding this comment

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

@corinnebosley I'm convinced that at this point, nd_values will always be an array (dask or numpy), as returned by _nd_points or _nd_bounds, and we've now added tests to implicitly check that, so I believe we're safe making this change (which is one of the sources of lazy data being realised)

nd_values = nd_values.reshape(shape)
else:
# If no coord, treat value as zero.
Expand Down
84 changes: 51 additions & 33 deletions lib/iris/tests/unit/aux_factory/test_AuxCoordFactory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,39 +26,48 @@
# importing anything else.
import iris.tests as tests

from iris._lazy_data import as_lazy_data, is_lazy_data
import numpy as np

import iris.coords
import iris
from iris._lazy_data import as_lazy_data, is_lazy_data
from iris.aux_factory import AuxCoordFactory
from iris.coords import AuxCoord


class Test__nd_points(tests.IrisTest):
def test_numpy_scalar_coord__zero_ndim(self):
points = np.array(1)
coord = AuxCoord(points)
result = AuxCoordFactory._nd_points(coord, (), 0)
expected = np.array([1])
self.assertArrayEqual(result, expected)

def test_numpy_scalar_coord(self):
points = np.arange(1)
coord = iris.coords.AuxCoord(points)
value = 1
points = np.array(value)
coord = AuxCoord(points)
result = AuxCoordFactory._nd_points(coord, (), 2)
expected = points[np.newaxis]
expected = np.array(value).reshape(1, 1)
self.assertArrayEqual(result, expected)

def test_numpy_simple(self):
points = np.arange(12).reshape(4, 3)
coord = iris.coords.AuxCoord(points)
coord = AuxCoord(points)
result = AuxCoordFactory._nd_points(coord, (0, 1), 2)
expected = points
self.assertArrayEqual(result, expected)

def test_numpy_complex(self):
points = np.arange(12).reshape(4, 3)
coord = iris.coords.AuxCoord(points)
coord = AuxCoord(points)
result = AuxCoordFactory._nd_points(coord, (3, 2), 5)
expected = points.T[np.newaxis, np.newaxis, ..., np.newaxis]
self.assertArrayEqual(result, expected)

def test_lazy_simple(self):
raw_points = np.arange(12).reshape(4, 3)
points = as_lazy_data(raw_points, 1)
coord = iris.coords.AuxCoord(points)
points = as_lazy_data(raw_points, raw_points.shape)
coord = AuxCoord(points)
self.assertTrue(is_lazy_data(coord.core_points()))
result = AuxCoordFactory._nd_points(coord, (0, 1), 2)
# Check we haven't triggered the loading of the coordinate values.
Expand All @@ -69,8 +78,8 @@ def test_lazy_simple(self):

def test_lazy_complex(self):
raw_points = np.arange(12).reshape(4, 3)
points = as_lazy_data(raw_points, 1)
coord = iris.coords.AuxCoord(points)
points = as_lazy_data(raw_points, raw_points.shape)
coord = AuxCoord(points)
self.assertTrue(is_lazy_data(coord.core_points()))
result = AuxCoordFactory._nd_points(coord, (3, 2), 5)
# Check we haven't triggered the loading of the coordinate values.
Expand All @@ -80,37 +89,45 @@ def test_lazy_complex(self):
self.assertArrayEqual(result, expected)



class Test__nd_bounds(tests.IrisTest):
def test_numpy_scalar_coord__zero_ndim(self):
points = np.array(0.5)
bounds = np.arange(2)
coord = AuxCoord(points, bounds=bounds)
result = AuxCoordFactory._nd_bounds(coord, (), 0)
expected = bounds
self.assertArrayEqual(result, expected)

def test_numpy_scalar_coord(self):
points = np.array(0.5)
bounds = np.arange(2).reshape(1, 2)
coord = iris.coords.AuxCoord(np.arange(1), bounds=bounds)
coord = AuxCoord(points, bounds=bounds)
result = AuxCoordFactory._nd_bounds(coord, (), 2)
expected = bounds[np.newaxis]
self.assertArrayEqual(result, expected)

def test_numpy_simple(self):
points = np.arange(12).reshape(4, 3)
bounds = np.arange(24).reshape(4, 3, 2)
coord = iris.coords.AuxCoord(points, bounds=bounds)
coord = AuxCoord(points, bounds=bounds)
result = AuxCoordFactory._nd_bounds(coord, (0, 1), 2)
expected = bounds
self.assertArrayEqual(result, expected)

def test_numpy_complex(self):
points = np.arange(12).reshape(4, 3)
bounds = np.arange(24).reshape(4, 3, 2)
coord = iris.coords.AuxCoord(points, bounds=bounds)
coord = AuxCoord(points, bounds=bounds)
result = AuxCoordFactory._nd_bounds(coord, (3, 2), 5)
expected = bounds.T[np.newaxis, np.newaxis, ..., np.newaxis]
expected = bounds.transpose((1, 0, 2)).reshape(1, 1, 3, 4, 1, 2)
self.assertArrayEqual(result, expected)

def test_lazy_simple(self):
raw_points = np.arange(12).reshape(4, 3)
points = as_lazy_data(raw_points, 1)
points = as_lazy_data(raw_points, raw_points.shape)
raw_bounds = np.arange(24).reshape(4, 3, 2)
bounds = as_lazy_data(raw_bounds, 1)
coord = iris.coords.AuxCoord(points, bounds=bounds)
bounds = as_lazy_data(raw_bounds, raw_bounds.shape)
coord = AuxCoord(points, bounds=bounds)
self.assertTrue(is_lazy_data(coord.core_bounds()))
result = AuxCoordFactory._nd_bounds(coord, (0, 1), 2)
# Check we haven't triggered the loading of the coordinate values.
Expand All @@ -121,40 +138,41 @@ def test_lazy_simple(self):

def test_lazy_complex(self):
raw_points = np.arange(12).reshape(4, 3)
points = as_lazy_data(raw_points, 1)
points = as_lazy_data(raw_points, raw_points.shape)
raw_bounds = np.arange(24).reshape(4, 3, 2)
bounds = as_lazy_data(raw_bounds, 1)
coord = iris.coords.AuxCoord(points, bounds=bounds)
bounds = as_lazy_data(raw_bounds, raw_bounds.shape)
coord = AuxCoord(points, bounds=bounds)
self.assertTrue(is_lazy_data(coord.core_bounds()))
result = AuxCoordFactory._nd_bounds(coord, (3, 2), 5)
# Check we haven't triggered the loading of the coordinate values.
self.assertTrue(is_lazy_data(coord.core_bounds()))
self.assertTrue(is_lazy_data(result))
expected = raw_bounds.T[np.newaxis, np.newaxis, ..., np.newaxis]
expected = raw_bounds.transpose((1, 0, 2)).reshape(1, 1, 3, 4, 1, 2)
self.assertArrayEqual(result, expected)


@tests.skip_data
class Test_lazy_aux_coords(tests.IrisTest):
def setUp(self):
self.cube = iris.load_cube(tests.get_data_path
(['NetCDF', 'testing',
'small_theta_colpex.nc']))
self.coords = self.cube.aux_coords + self.cube.derived_coords

def test_lazy_coord_loading(self):
# Test that points and bounds arrays stay lazy upon cube loading
for coord in self.cube.aux_coords and self.cube.derived_coords:
def _check_lazy(self):
for coord in self.coords:
self.assertTrue(coord.has_lazy_points())
if coord.has_bounds():
self.assertTrue(coord.has_lazy_bounds())

def test_lazy_coord_loading(self):
# Test that points and bounds arrays stay lazy upon cube loading.
self._check_lazy()

def test_lazy_coord_printing(self):
# Test that points and bounds arrays stay lazy upon cube printing
cube = self.cube
printed_cube = str(cube)
for coord in cube.aux_coords and cube.derived_coords:
self.assertTrue(coord.has_lazy_points())
if coord.has_bounds():
self.assertTrue(coord.has_lazy_bounds())
# Test that points and bounds arrays stay lazy after cube printing.
_ = str(self.cube)
self._check_lazy()


if __name__ == '__main__':
Expand Down