Data first #2392

marqh · 2017-02-20T11:17:20Z

for discussion regarding how to implement this interface

marqh · 2017-02-21T16:49:33Z

lib/iris/fileformats/netcdf.py

-    data = iris._lazy_data.as_lazy_data(proxy)
-    cube = iris.cube.Cube(data)
+    data = da.from_array(proxy, chunks=100)
+    cube = iris.cube.Cube(data, fill_value=fill_value)


should this pass the data type in as well?

marqh · 2017-02-21T16:51:10Z

lib/iris/fileformats/rules.py

    cube = iris.cube.Cube(data,
                          attributes=metadata.attributes,
                          cell_methods=metadata.cell_methods,
                          dim_coords_and_dims=metadata.dim_coords_and_dims,
-                          aux_coords_and_dims=metadata.aux_coords_and_dims)
+                          aux_coords_and_dims=metadata.aux_coords_and_dims,
+                          fill_value=field.bmdi)


should this also set the data type

marqh · 2017-02-21T16:51:42Z

lib/iris/fileformats/rules.py

@@ -901,15 +902,15 @@ def _make_cube(field, converter):
    metadata = converter(field)



please re-implement this, don't try/accept

marqh · 2017-02-21T17:08:43Z

lib/iris/cube.py

@@ -1631,9 +1636,10 @@ def lazy_data(self, array=None):
            A lazy array, representing the Cube data array.

        """
+        result = None


don't allow this, if there is not _numpy_array and no _dask_array raise an exception

bjlittle · 2017-02-22T18:03:45Z

lib/iris/tests/unit/fileformats/pp/test__data_bytes_to_shaped_array.py

                                                  self.create_lbpack(lbpack),
                                                  None,
                                                  mask.shape, np.dtype('>f4'),
                                                  -999, mask=mask)
+        return np.ma.masked_array(data, np.isnan(data), fill_value=-999)


np.ma -> ma

bjlittle · 2017-02-22T18:04:08Z

lib/iris/tests/unit/lazy_data/test_array_masked_to_nans.py

@@ -34,7 +34,7 @@ def test_masked(self):
        masked_array = np.ma.masked_array([[1.0, 2.0], [3.0, 4.0]],


np.ma -> ma

bjlittle · 2017-02-22T18:04:25Z

lib/iris/tests/unit/lazy_data/test_array_masked_to_nans.py

@@ -34,7 +34,7 @@ def test_masked(self):
        masked_array = np.ma.masked_array([[1.0, 2.0], [3.0, 4.0]],
                                          mask=[[0, 1], [0, 0]])

-        result = array_masked_to_nans(masked_array)
+        result = array_masked_to_nans(masked_array).data

        self.assertIsInstance(result, np.ndarray)
        self.assertFalse(isinstance(result, np.ma.MaskedArray))


np.ma -> ma

bjlittle · 2017-02-22T18:04:44Z

lib/iris/tests/unit/lazy_data/test_array_masked_to_nans.py

@@ -48,7 +48,7 @@ def test_masked(self):
    def test_empty_mask(self):
        masked_array = np.ma.masked_array([1.0, 2.0], mask=[0, 0])


np.ma -> ma

bjlittle · 2017-02-22T18:05:02Z

lib/iris/tests/unit/lazy_data/test_array_masked_to_nans.py

@@ -48,7 +48,7 @@ def test_masked(self):
    def test_empty_mask(self):
        masked_array = np.ma.masked_array([1.0, 2.0], mask=[0, 0])

-        result = array_masked_to_nans(masked_array)
+        result = array_masked_to_nans(masked_array).data

        self.assertIsInstance(result, np.ndarray)
        self.assertFalse(isinstance(result, np.ma.MaskedArray))


np.ma -> ma

bjlittle · 2017-02-22T19:15:19Z

lib/iris/_merge.py

 import iris.util
+from iris._lazy_data import is_lazy_data, array_masked_to_nans


@marqh Minor point but from iris._lazy ... sorts before alphabetical, and should be the first iris import.

bjlittle · 2017-02-22T19:17:26Z

lib/iris/cube.py

 import iris._merge
 import iris.exceptions
 import iris.util
+from iris._lazy_data import is_lazy_data, array_masked_to_nans


@marqh the import order is wrong

bjlittle · 2017-02-22T19:26:26Z

lib/iris/cube.py


    def has_lazy_data(self):
-        return is_lazy_data(self._my_data)
+        return True if self._numpy_array is None else False


@marqh I'd prefer

return self._numpy_array is None

Also, isn't it the case the both cube._numpy_array and cube._dask_array could be populated? If so, are we saying that if that's the case, the cube isn't lazy?

Is there an explicit statement to define whether it's valid for both of these attributes to be populated? Sutely, it's totally invalid for both not to be populated ... do we have checks and balances for that to maintain that integrity within the cube?

Also, isn't it the case the both cube._numpy_array and cube._dask_array could be populated?
yes, at present this is the case
If so, are we saying that if that's the case, the cube isn't lazy?
yes, it is not lazy
if cube._numpy_array exists it is always used in preference

Is there an explicit statement to define whether it's valid for both of these attributes to be populated?
No, there is no explicit statement. It is an open question.
We should answer it. The question here is should we answer it on this PR or raise an issue and answer it?
The utility of having both is the cube._numpy_array may be set to None, reverting the cube back to it's initally loaded state, with a file proxy. I don't know how valuable this may be.

Sutely, it's totally invalid for both not to be populated ...

Yes, that is invalid and exceptions are thrown

do we have checks and balances for that to maintain that integrity within the cube?

there are some, are there enough?

marqh · 2017-02-23T09:21:15Z

@bjlittle

ok, further review actions pushed. hopefully i've maintained test success; we shall see

please squash and merge at will once travis is content

bjlittle · 2017-02-23T09:26:24Z

@marqh Agreed.

If the tests pass, then let's bank this PR. It's a feature branch and we need to move forwards 👍

bjlittle · 2017-02-23T09:30:56Z

Ping @marqh you still have a couple of tests referencing the now nuked _data_cache in PP land ...

marqh · 2017-02-23T09:41:37Z

Ping @marqh you still have a couple of tests referencing the now nuked _data_cache ...

doh
trying again

marqh · 2017-02-23T10:41:36Z

@bjlittle
it has gone green
...

DPeterK · 2017-02-23T10:46:28Z

Woop! 🎉 🍰 🍷

Core refactor for dask usage

pp-mo · 2017-02-23T17:52:03Z

lib/iris/tests/integration/test_pp.py

@@ -47,6 +47,8 @@ def _test_coord(self, cube, point, bounds=None, **kwargs):
        if bounds is not None:
            self.assertArrayEqual(coords[0].bounds, [bounds])

+    # hits a segfault, very odd


hits a segfault

I think these may be fixed by : #2395
Short version: since we changed our test for lazy data, a simple Mock tests as lazy instead of real.
Then np.ma.asarray() can hit an endless loop (whereas np.asarray was ok).

Core refactor for dask usage

pp-mo · 2017-05-16T14:49:11Z

lib/iris/fileformats/pp.py

+        # The proxy supplies nan filled arrays and caches data.
+        data = self._data[...]
+        if data.dtype.kind == 'i' and self.bmdi == -1e30:
+            self.bmdi = -9999


Magic number ?

Core refactor for dask usage

marqh added 12 commits February 16, 2017 14:47

not my data

667a191

missing data and fill values

dd593e8

graph array reference

d519ea3

b0rked referencing

02ec854

make data primate once called

eee7580

fill_value not done yet

beb5189

fill value handling and naming

10704cf

_my_data is always masked, data is not

a7c79f3

retire _lazy_data

ad1ad55

Merge branch 'dmerge' into data_first

aa0d9d7

ensure proxies are wrapped in dask arrays

966af7e

developer docs

59dcb80

marqh added the Status: Work in Progress label Feb 20, 2017

pp-mo mentioned this pull request Feb 20, 2017

Review cubedata interface #2393

Closed

make data_graph private

6c08ae4

QuLogic added this to the dask milestone Feb 20, 2017

marqh added 8 commits February 21, 2017 09:55

array types, ensure nans in lazy array

4e5ec49

avoid copying

235ef13

mask handling

3567369

centralise is_dask_array

d182e72

rename; _my_data: _numpy_array ; _data_graph: _dask_array

1755ede

util.is_dask_array

18795ad

coding standards

4f21dc7

fill_value, dtype, shape on cube; proxies always return nan arrays

96d60ae

marqh commented Feb 21, 2017

View reviewed changes

test bug fixing

5716327

bjlittle self-assigned this Feb 21, 2017

bjlittle reviewed Feb 22, 2017

View reviewed changes

review actions

3586a28

removed pp data_cache

ca61115

skip mo_pack patch

02ff5ba

marqh force-pushed the data_first branch from 539f058 to 02ff5ba Compare February 23, 2017 10:31

bjlittle merged commit b71bd27 into SciTools:dask Feb 23, 2017

bjlittle removed the Status: Work in Progress label Feb 23, 2017

marqh added a commit to marqh/iris that referenced this pull request Feb 23, 2017

Data first (SciTools#2392)

d2116d9

Core refactor for dask usage

pp-mo reviewed Feb 23, 2017

View reviewed changes

marqh added a commit to marqh/iris that referenced this pull request Feb 24, 2017

Data first (SciTools#2392)

2af3603

Core refactor for dask usage

marqh mentioned this pull request Mar 9, 2017

Tighten purpose of array_masked_to_nans #2424

Merged

pp-mo reviewed May 16, 2017

View reviewed changes

bjlittle pushed a commit to bjlittle/iris that referenced this pull request May 31, 2017

Data first (SciTools#2392)

da3b947

Core refactor for dask usage

QuLogic modified the milestones: dask, v2.0 Aug 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data first #2392

Data first #2392

marqh commented Feb 20, 2017

marqh Feb 21, 2017

marqh Feb 21, 2017

marqh Feb 21, 2017

marqh Feb 21, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017

bjlittle Feb 22, 2017 •

edited

Loading

marqh Feb 23, 2017

marqh commented Feb 23, 2017

bjlittle commented Feb 23, 2017

bjlittle commented Feb 23, 2017 •

edited

Loading

marqh commented Feb 23, 2017

marqh commented Feb 23, 2017

DPeterK commented Feb 23, 2017

pp-mo Feb 23, 2017 •

edited

Loading

pp-mo May 16, 2017

		@@ -901,15 +902,15 @@ def _make_cube(field, converter):
		metadata = converter(field)

		@@ -34,7 +34,7 @@ def test_masked(self):
		masked_array = np.ma.masked_array([[1.0, 2.0], [3.0, 4.0]],

		@@ -48,7 +48,7 @@ def test_masked(self):
		def test_empty_mask(self):
		masked_array = np.ma.masked_array([1.0, 2.0], mask=[0, 0])

		import iris.util
		from iris._lazy_data import is_lazy_data, array_masked_to_nans

Data first #2392

Data first #2392

Conversation

marqh commented Feb 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marqh commented Feb 23, 2017

bjlittle commented Feb 23, 2017

bjlittle commented Feb 23, 2017 • edited Loading

marqh commented Feb 23, 2017

marqh commented Feb 23, 2017

DPeterK commented Feb 23, 2017

pp-mo Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Feb 22, 2017 •

edited

Loading

bjlittle commented Feb 23, 2017 •

edited

Loading

pp-mo Feb 23, 2017 •

edited

Loading