Read mitgrid files #114

raphaeldussin · 2019-01-06T02:38:22Z

fixes #14

tested with llc90 and aste. there is room to add cube sphere support once #91 is resolved
and it would be also easy to make it work with simpler geometries, although I have not seen
yet the case of users using input grid files in those.

stickler-ci · 2019-01-06T02:38:37Z

xmitgcm/utils.py

+        gridfile must contain <NFACET> as wildcard (e.g. tile<NFACET>.mitgrid)
+    nx:     int
+        size of the face in the x direction
+    ny:     int


E501 line too long (81 > 79 characters)

stickler-ci · 2019-01-06T02:38:37Z

xmitgcm/utils.py

+    # create placeholders for data
+    gridfields= {}
+    for field in file_metadata['fldList']:
+        gridfields.update({field: np.zeros(shape_out)})


F841 local variable 'shape' is assigned to but never used

stickler-ci · 2019-01-06T02:38:37Z

xmitgcm/utils.py

+                nygrid = file_metadata['nx'] + 1
+
+            grid_metadata.update({'nx': nxgrid, 'ny': nygrid,
+                                  'has_faces': False})


E501 line too long (80 > 79 characters)

stickler-ci · 2019-01-06T02:38:50Z

xmitgcm/utils.py

+    gridfile: str
+        gridfile must contain <NFACET> as wildcard (e.g. tile<NFACET>.mitgrid)
+    nx:     int
+        size of the face in the x direction


E501 line too long (81 > 79 characters)

stickler-ci · 2019-01-06T02:38:50Z

xmitgcm/utils.py

+
+    # create placeholders for data
+    gridfields = {}
+    for field in file_metadata['fldList']:


F841 local variable 'shape' is assigned to but never used

stickler-ci · 2019-01-06T02:38:50Z

xmitgcm/utils.py

+                nxgrid = file_metadata['ny_facets'][kfacet] + 1
+                nygrid = file_metadata['nx'] + 1
+
+            grid_metadata.update({'nx': nxgrid, 'ny': nygrid,


E501 line too long (80 > 79 characters)

codecov-io · 2019-01-06T02:43:00Z

Codecov Report

Merging #114 into master will increase coverage by 1.3%.
The diff coverage is 98.33%.

@@            Coverage Diff            @@
##           master     #114     +/-   ##
=========================================
+ Coverage   92.28%   93.59%   +1.3%     
=========================================
  Files           4        4             
  Lines         830      890     +60     
  Branches      190      203     +13     
=========================================
+ Hits          766      833     +67     
+ Misses         38       32      -6     
+ Partials       26       25      -1

Impacted Files	Coverage Δ
xmitgcm/utils.py	`94.85% <98.33%> (+2.6%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 421c4c5...9be3655. Read the comment docs.

stickler-ci · 2019-01-06T02:43:29Z

xmitgcm/utils.py

+    gridfile: str
+        gridfile must contain <NFACET> as wildcard (e.g. tile<NFACET>.mitgrid)
+    nx:     int
+        size of the face in the x direction


W291 trailing whitespace

rabernat · 2019-01-08T10:19:29Z

This looks very useful!

Tests and documentation are necessary.

rabernat · 2019-01-08T10:22:33Z

xmitgcm/utils.py

+                       'RAS': (['face', 'j_g', 'i'],   gridfields['RAS']),
+                       'DXG': (['face', 'j_g', 'i'],   gridfields['DXG']),
+                       'DYG': (['face', 'j', 'i_g'],   gridfields['DYG'])
+                       })


This dataset is missing the coordinates for j and i. In a regular xmitgcm dataset, these variables have explicit coordinates.

rabernat · 2019-01-08T10:24:26Z

xmitgcm/utils.py

+
+            for field in file_metadata['fldList']:
+                # symetrize
+                tmp = rawfields[field][:, :, :-1, :-1].squeeze()


It occurs to me that we are potentially throwing away some useful information here. One thing that I dislike about MDS output is that it throws away the outer cell face point. Might be worth having an option to keep those outer elements for variables on the u and v points.

it's all zeros so no worries.

import numpy as np all = np.fromfile('tile003.mitgrid','>d') XC = all[:91*91] XC = np.reshape(XC,(91,91))

In [14]: XC[-1,:]
Out[14]:
array([0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0.])

In [15]: XC[:,-1]
Out[15]:
array([0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0.])

rabernat · 2019-01-08T10:25:42Z

xmitgcm/utils.py

+                        if file_metadata['transpose_face'][face]:
+                            dataface = dataface.transpose()
+                        # assign values
+                        gridfields[field][face, :, :] = dataface


Doesn't this same logic already live elsewhere in xmitgcm? If so, it's probably better to re-call that function.

I haven't seen anywhere in xmitgcm functions working on multi-files but I can have missed it.

Does this work with dask?

xmitgcm/utils.py

rabernat · 2019-01-08T10:27:38Z

xmitgcm/utils.py

+            grid_metadata.update({'nx': nxgrid, 'ny': nygrid,
+                                  'has_faces': False})
+
+            raw = read_all_variables(grid_metadata['vars'], grid_metadata)


Is there a dask option here?

I'm thinking about what will happen if I run this on the LLC4320 mitgrid files.

stickler-ci · 2019-01-11T03:18:37Z

xmitgcm/test/test_utils.py

+                             use_dask=False,
+                             extra_metadata=md)
+    assert type(ds) == xarray.Dataset
+    assert ds['XC'].values.shape == expected['shape']


F405 'xarray' may be undefined, or defined from star imports: xmitgcm.test.test_xmitgcm_common

maybe add a check here that all the expected variables are in ds?

Also need to check that use_dask works as expected and doesn't trigger eagerreading of the data.

xmitgcm/test/test_xmitgcm_common.py

stickler-ci · 2019-01-11T03:18:51Z

xmitgcm/test/test_utils.py

+                             precision='double', endian='>',
+                             use_dask=False,
+                             extra_metadata=md)
+    assert type(ds) == xarray.Dataset


F405 'xarray' may be undefined, or defined from star imports: xmitgcm.test.test_xmitgcm_common

raphaeldussin · 2019-01-11T03:28:15Z

archive for grid tests are on figshare. Build will work when #115 is merged

stickler-ci · 2019-01-14T18:02:27Z

xmitgcm/test/test_utils.py

+
+    # passing llc without metadata should fail
+    if expected['geometry'] == 'llc':
+        with pytest.raises(ValueError):


F405 'pytest' may be undefined, or defined from star imports: xmitgcm.test.test_xmitgcm_common

xmitgcm/test/test_utils.py

raphaeldussin · 2019-01-14T18:34:32Z

@rabernat strickler-ci is really nitpicking, can we merge this anyway? I will fill in the blanks with the cube sphere grid in the cs branch. Not very orthogonal here I'm sorry!

rabernat

Looking good. Just want to verify that the use_dask option actually works as described. So that needs a test.

rabernat · 2019-01-14T18:38:41Z

xmitgcm/test/test_utils.py

+                             use_dask=False,
+                             extra_metadata=md)
+    assert type(ds) == xarray.Dataset
+    assert ds['XC'].values.shape == expected['shape']


maybe add a check here that all the expected variables are in ds?

rabernat · 2019-01-14T18:44:19Z

xmitgcm/utils.py

+        gridfields.update({field: np.zeros(shape)})
+
+    if geometry == 'llc':
+        for kfacet in np.arange(nfacets):


range(nfacets) would be better here

rabernat · 2019-01-14T18:44:54Z

xmitgcm/utils.py

+                        if file_metadata['transpose_face'][face]:
+                            dataface = dataface.transpose()
+                        # assign values
+                        gridfields[field][face, :, :] = dataface


Does this work with dask?

rabernat · 2019-01-14T18:45:24Z

xmitgcm/test/test_utils.py

+                             use_dask=False,
+                             extra_metadata=md)
+    assert type(ds) == xarray.Dataset
+    assert ds['XC'].values.shape == expected['shape']


Also need to check that use_dask works as expected and doesn't trigger eagerreading of the data.

stickler-ci · 2019-01-14T21:00:17Z

xmitgcm/test/test_utils.py

@@ -843,3 +844,39 @@ def test_get_extra_metadata(domain, nx):

    with pytest.raises(ValueError):
        em = get_extra_metadata(domain='notinlist', nx=nx)
+
+
+@pytest.mark.parametrize("usedask", [True, False])


F405 'pytest' may be undefined, or defined from star imports: xmitgcm.test.test_xmitgcm_common

stickler-ci · 2019-01-14T21:00:17Z

xmitgcm/utils.py

+    # create placeholders for data
+    gridfields = {}
+    for field in file_metadata['fldList']:
+        gridfields.update({field: None})


F841 local variable 'shape' is assigned to but never used

stickler-ci · 2019-01-14T21:00:17Z

xmitgcm/utils.py

+    gridfields = {}
+    for field in file_metadata['fldList']:
+        gridfields.update({field: None})
+


F841 local variable 'chunks' is assigned to but never used

raphaeldussin · 2019-01-14T21:50:12Z

@rabernat dask should work with global llc grids (not regional because of the padding), the dsa.concatenate on l1260 in utils.py makes the results being a dask array wether the input is a dask or numpy array so I can't test for the type in the output.

rabernat · 2019-01-15T11:35:22Z

dsa.concatenate on l1260 in utils.py makes the results being a dask array wether the input is a dask or numpy array so I can't test for the type in the output.

Ok, then maybe we can come back to this in a future PR. I would prefer to use a version of concatenate that returns numpy arrays for numpy inputs. We have something like that in xgcm:
https://github.com/xgcm/xgcm/blob/master/xgcm/duck_array_ops.py#L37

* start working on reading grid * read ext grid * works for llc * works with aste+docstring * Fixing style errors. * fix cosmetics * fix cosmetics * fixes * Fixing style errors. * testing * Fixing style errors. * fix strickler * add link/md5 * fix strickler * more testing * Fixing style errors. * fix strickler * fix strickler * make dask lazy * Fixing style errors. * fix from strickler

stickler-ci reviewed Jan 6, 2019

View reviewed changes

rabernat reviewed Jan 8, 2019

View reviewed changes

xmitgcm/utils.py Show resolved Hide resolved

rabernat reviewed Jan 8, 2019

View reviewed changes

stickler-ci reviewed Jan 11, 2019

View reviewed changes

raphaeldussin and others added 12 commits January 14, 2019 11:28

start working on reading grid

c7a5626

read ext grid

c7c83a7

works for llc

9664363

works with aste+docstring

d2a3965

Fixing style errors.

9c59132

fix cosmetics

b66448d

fix cosmetics

9264562

fixes

09ecd92

Fixing style errors.

fe6e225

testing

67f5f1d

Fixing style errors.

971ee7b

fix strickler

df9b9ca

raphaeldussin force-pushed the read_grid branch from 281f0bb to df9b9ca Compare January 14, 2019 16:36

raphaeldussin added 3 commits January 14, 2019 11:50

add link/md5

ba81a82

fix strickler

e4efd7b

more testing

5caf237

stickler-ci reviewed Jan 14, 2019

View reviewed changes

stickler-ci and others added 2 commits January 14, 2019 18:02

Fixing style errors.

f747e65

fix strickler

3763a44

stickler-ci reviewed Jan 14, 2019

View reviewed changes

fix strickler

725a794

rabernat requested changes Jan 14, 2019

View reviewed changes

make dask lazy

449a485

stickler-ci reviewed Jan 14, 2019

View reviewed changes

stickler-ci and others added 2 commits January 14, 2019 21:00

Fixing style errors.

5187bcd

fix from strickler

9be3655

rabernat approved these changes Jan 15, 2019

View reviewed changes

rabernat merged commit 8a6cf12 into MITgcm:master Jan 15, 2019

raphaeldussin deleted the read_grid branch January 15, 2019 16:11

rabernat mentioned this pull request Mar 21, 2019

collaborate with xmitgcm? ECCO-GROUP/ECCOv4-py#6

Closed

Read mitgrid files #114

Read mitgrid files #114

Conversation

raphaeldussin commented Jan 6, 2019

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

codecov-io commented Jan 6, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

rabernat commented Jan 8, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphaeldussin commented Jan 11, 2019

Choose a reason for hiding this comment

raphaeldussin commented Jan 14, 2019

rabernat left a 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

raphaeldussin commented Jan 14, 2019

rabernat commented Jan 15, 2019

codecov-io commented Jan 6, 2019 •

edited

Loading