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.

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

Jump to bottom

Fix master np113 #2616

Merged
merged 5 commits into from
Jun 24, 2017
Merged

Fix master np113 #2616

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca4fabf
Fix np113 meshgrid output dtype change.
bjlittle Jun 22, 2017
73dbba0
Fix np113 array2string formatter option.
bjlittle Jun 22, 2017
3ab0d26
Fix data manager unit test.
bjlittle Jun 22, 2017
5a6fba0
PEP440 version
bjlittle Jun 22, 2017
4b7a191
Review actions.
bjlittle Jun 23, 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 lib/iris/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def callback(cube, field, filename):


# Iris revision.
__version__ = '2.0.0-DEV'
__version__ = '2.0.dev0'
Copy link
Member

Choose a reason for hiding this comment

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

Can you maybe explain exactly what this is about, just for future reference ?

Copy link
Member Author

Choose a reason for hiding this comment

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

To be fair, this doesn't belong in this PR, so I'll pull it ... but the __version__ should be PEP440.

I'll save that for another day and another PR 👍


# Restrict the names imported when using "from iris import *"
__all__ = ['load', 'load_cube', 'load_cubes', 'load_raw',
Expand Down
3 changes: 2 additions & 1 deletion lib/iris/analysis/_regrid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
extend_circular_coord_and_data,
get_xy_dim_coords, snapshot_grid)
from iris.analysis._scipy_interpolate import _RegularGridInterpolator
from iris.analysis.cartography import _meshgrid
import iris.cube


Expand Down Expand Up @@ -124,7 +125,7 @@ def _sample_grid(src_coord_system, grid_x_coord, grid_y_coord):
arrays.

"""
grid_x, grid_y = np.meshgrid(grid_x_coord.points, grid_y_coord.points)
grid_x, grid_y = _meshgrid(grid_x_coord.points, grid_y_coord.points)
# Skip the CRS transform if we can to avoid precision problems.
if src_coord_system == grid_x_coord.coord_system:
sample_grid_x = grid_x
Expand Down
26 changes: 22 additions & 4 deletions lib/iris/analysis/cartography.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,24 @@ def _xy_range(cube, mode=None):
return (x_range, y_range)


def _meshgrid(x, y):
Copy link
Member

Choose a reason for hiding this comment

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

I think on balance I'd prefer this to live in iris.util, but only slightly -- your call.
Please do keep it private !

Copy link

Choose a reason for hiding this comment

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

Agreed. Is this not behaviour that would be useful to have consistent across the entire package?

Perhaps if util were made a sub-package, then it would no longer be such a difficult call to add add additional 'utilities' for fear of further discontinuity:

iris.util.ndarray.meshgrid

In ANTS we have the following structure, which has served us very well:

ants.utils.ndarray
ants.utils.coord
ants.utils.cube

Copy link
Member

Choose a reason for hiding this comment

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

Not looked at the details, but numpy/numpy#5302 refers.

"""
@numpy v1.13, the dtype of each output nD coordinate is the same as its
associated input 1D coordinate. This is not the case prior to numpy v1.13,
where the output dtype is cast up to its highest resolution, regardlessly.

This convenience function ensures consistent meshgrid behaviour across
numpy versions.

"""
mx, my = np.meshgrid(x, y)
if mx.dtype != x.dtype:
mx = mx.astype(x.dtype)
if my.dtype != y.dtype:
my = my.astype(y.dtype)
return mx, my


def get_xy_grids(cube):
"""
Return 2D X and Y points for a given cube.
Expand All @@ -259,7 +277,7 @@ def get_xy_grids(cube):

if x.ndim == y.ndim == 1:
# Convert to 2D.
x, y = np.meshgrid(x, y)
x, y = _meshgrid(x, y)
elif x.ndim == y.ndim == 2:
# They are already in the correct shape.
pass
Expand All @@ -284,7 +302,7 @@ def get_xy_contiguous_bounded_grids(cube):

x = x_coord.contiguous_bounds()
y = y_coord.contiguous_bounds()
x, y = np.meshgrid(x, y)
x, y = _meshgrid(x, y)

return (x, y)

Expand Down Expand Up @@ -608,7 +626,7 @@ def project(cube, target_proj, nx=None, ny=None):
source_x = lon_coord.points
source_y = lat_coord.points
if source_x.ndim != 2 or source_y.ndim != 2:
source_x, source_y = np.meshgrid(source_x, source_y)
source_x, source_y = _meshgrid(source_x, source_y)

# Calculate target grid
target_cs = None
Expand Down Expand Up @@ -1062,7 +1080,7 @@ def rotate_winds(u_cube, v_cube, target_cs):

# Convert points to 2D, if not already, and determine dims.
if x.ndim == y.ndim == 1:
x, y = np.meshgrid(x, y)
x, y = _meshgrid(x, y)
dims = (y_dims[0], x_dims[0])
else:
dims = x_dims
Expand Down
3 changes: 2 additions & 1 deletion lib/iris/analysis/trajectory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from iris.analysis._interpolate_private import \
_nearest_neighbour_indices_ndcoords, linear as linear_regrid
from iris.analysis._interpolation import snapshot_grid
from iris.analysis.cartography import _meshgrid


class _Segment(object):
Expand Down Expand Up @@ -504,7 +505,7 @@ def __init__(self, src_cube, target_grid_cube):
self.tgt_grid_shape = tgt_y_coord.shape + tgt_x_coord.shape

# Calculate sample points as 2d arrays, like broadcast (NY,1)*(1,NX).
x_2d, y_2d = np.meshgrid(tgt_x_coord.points, tgt_y_coord.points)
x_2d, y_2d = _meshgrid(tgt_x_coord.points, tgt_y_coord.points)
# Cast as a "trajectory", to suit the method used.
self.trajectory = ((tgt_x_coord.name(), x_2d.flatten()),
(tgt_y_coord.name(), y_2d.flatten()))
Expand Down
2 changes: 1 addition & 1 deletion lib/iris/coords.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ def _str_dates(self, dates_as_numbers):
date_obj_array = self.units.num2date(dates_as_numbers)
kwargs = {'separator': ', ', 'prefix': ' '}
return np.core.arrayprint.array2string(date_obj_array,
formatter={'numpystr': str},
formatter={'all': str},
**kwargs)

def __str__(self):
Expand Down
6 changes: 4 additions & 2 deletions lib/iris/experimental/regrid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,8 @@ def regrid_area_weighted_rectilinear_src_and_grid(src_cube, grid_cube,

# Wrap up the data as a Cube.
# Create 2d meshgrids as required by _create_cube func.
meshgrid_x, meshgrid_y = np.meshgrid(grid_x.points, grid_y.points)
meshgrid_x, meshgrid_y = iris.analysis.cartography._meshgrid(grid_x.points,
grid_y.points)
regrid_callback = RectilinearRegridder._regrid
new_cube = RectilinearRegridder._create_cube(new_data, src_cube,
src_x_dim, src_y_dim,
Expand Down Expand Up @@ -1421,7 +1422,8 @@ def _regrid(src_data, xy_dim, src_x_coord, src_y_coord,
src_projection, src_x_coord.points, src_y_coord.points)

tgt_projection = tgt_x_coord.coord_system.as_cartopy_projection()
tgt_x, tgt_y = np.meshgrid(tgt_x_coord.points, tgt_y_coord.points)
tgt_x, tgt_y = iris.analysis.cartography._meshgrid(tgt_x_coord.points,
tgt_y_coord.points)
projected_tgt_grid = projection.transform_points(
tgt_projection, tgt_x, tgt_y)

Expand Down
11 changes: 6 additions & 5 deletions lib/iris/experimental/regrid_conservative.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 All @@ -25,9 +25,10 @@
import cartopy.crs as ccrs
import numpy as np

from iris.analysis._interpolation import get_xy_dim_coords
import iris
from iris.analysis._interpolation import get_xy_dim_coords
from iris.analysis._regrid import RectilinearRegridder
from iris.analysis.cartography import _meshgrid


#: A static Cartopy Geodetic() instance for transforming to true-lat-lons.
Expand Down Expand Up @@ -81,8 +82,8 @@ def _make_esmpy_field(x_coord, y_coord, ref_name='field',
grid = ESMF.Grid(dims)

# Get all cell corner coordinates as true-lat-lons
x_bounds, y_bounds = np.meshgrid(x_coord.contiguous_bounds(),
y_coord.contiguous_bounds())
x_bounds, y_bounds = _meshgrid(x_coord.contiguous_bounds(),
y_coord.contiguous_bounds())
grid_crs = x_coord.coord_system.as_cartopy_crs()
lon_bounds, lat_bounds = _convert_latlons(grid_crs, x_bounds, y_bounds)

Expand Down Expand Up @@ -113,7 +114,7 @@ def _make_esmpy_field(x_coord, y_coord, ref_name='field',
x_centres = 0.5 * (x_centres[:-1] + x_centres[1:])
y_centres = y_coord.contiguous_bounds()
y_centres = 0.5 * (y_centres[:-1] + y_centres[1:])
x_points, y_points = np.meshgrid(x_centres, y_centres)
x_points, y_points = _meshgrid(x_centres, y_centres)
lon_points, lat_points = _convert_latlons(grid_crs, x_points, y_points)

# Add grid 'coord' element for centres + fill with centre-points values.
Expand Down
2 changes: 1 addition & 1 deletion lib/iris/tests/results/analysis/rotated_pole.0.rlat.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"std": 3.7195861803241614, "min": -5.593200206756592, "max": 8.72130012512207, "shape": [93, 75], "masked": false, "mean": 1.6348200228305594}
{"std": 3.7195863723754883, "min": -5.5932002067565918, "max": 8.7213001251220703, "shape": [93, 75], "masked": false, "mean": 1.6348202228546143}
Copy link
Member

@pp-mo pp-mo Jun 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It might have been "nicer" here to reduce the tolerance on the test for when it happens again (!).
I see these values only just fail the test at a relative tolerance of 1e-7, as default for assertArrayAllClose (+hence assertDataAlmostEqual) : mean value relative change is approx 1.22e-7.
You could maybe add something like , reltol=5.0e-6 to the 4 calls here
You could do that instead, or as well if you prefer (changes are very small after all).

Just a suggestion -- skip if you don't want.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pp-mo I'm really against fudging the tolerances. We took that step previously with testing and it ultimately lead to a bad place, particularly with graphical testing.

So, I'd rather not take that option, unless you consider it unwise.

Copy link
Member

Choose a reason for hiding this comment

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

I don't consider it fudging as long as the value used remains appropriate.
I know that was not always the case for our image comparison tolerance, where it got stupidly huge for a while... !
The default value of 1e-7 is designed to cope with float32 rounding errors, but can easily be a bit fine for testing other calculated values, like coordinate projections.
For instance, I just encountered some other tests failing at that level when I was messing about with different dask chunkings.

2 changes: 1 addition & 1 deletion lib/iris/tests/unit/data_manager/test_DataManager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ def test_with_lazy_mask_array__masked(self):
self.assertIsInstance(result, ma.MaskedArray)
self.assertIsNone(dm._realised_dtype)
self.assertEqual(dm.dtype, self.realised_dtype)
self.assertArrayEqual(result, self.lazy_mask_array_masked.compute())
self.assertArrayEqual(result, self.mask_array_masked)

def test_with_real_masked_constant(self):
masked_data = ma.masked_array([666], mask=True, dtype=np.dtype('f8'))
Expand Down