JiaweiZhuang / xESMF

Universal Regridder for Geospatial Data
http://xesmf.readthedocs.io/
MIT License
269 stars 49 forks source link

Input array is not F_CONTIGUOUS #25

Open leifdenby opened 6 years ago

leifdenby commented 6 years ago

The issue I am having is that I am getting a warning when creating a regridder (with xesmf.Regridder) that the input is not Fortran contiguous. Independent of what I do with the input the warning always appears.

Digging through the code I can see that the lat and lon variables are always transposed (to make them Fortran contiguous), but because as_2d_mesh simply passes the lat/lon arrays through if they are already 2D then these arrays might already be Fortran contiguous, and so transposing them will make them C contiguous instead. My suggestion would be to in ds_to_ESMFgrid only to transpose if necessary, or to use np.asfortranarray instead to ensure the data is always Fortran contiguous.

I've put a jupyter notebook here to demonstrate the issue: https://gist.github.com/leifdenby/a91b616ce5fb077e44d28556208626b4

If this sounds reasonable I'd be happy to make a pull requests which would include tests to ensure this change doesn't break anything else.

Thanks for a great tool btw! Love how easy it is to use :)

leifdenby commented 6 years ago

Ps. I can of course do (in the second example in my notebook)

ds_in['lat'] = (('x', 'y'), np.asfortranarray(lat).T)
ds_in['lon'] = (('x', 'y'), np.asfortranarray(lon).T)

so that when the transpose back is done inside ds_to_ESMFgrid we get back a Fortran contiguous array, but it just feels a bit hacky to me, but maybe I'm overthinking this?

JiaweiZhuang commented 6 years ago

Thanks for reporting and sorry for the confusing warning message. In short, simply pass C-ordered array (newly created array without .T, or most data read from files) to xesmf.Regridder(), then the warning will disappear (like the first case in your notebook)

I added this warning because ESMPy prefer F-ordered array: nawendt/esmpy-tutorial#4. But as you notice, all arrays are pre-transposed in xESMF code, so the user API actually expects C-order. I didn't expect users to see this "internal warning" if they always use normal, C-ordered numpy arrays.

The performance impact in this case is quite negligible so I'd consider removing this warning. I'll probably only keep the memory-layout check for input data (not grids), which does have a noticeable impact on performance.

(Glad that you find xESMF helpful!)