Regression in addcyclic behaviour

[ajdawson]

Pull request #138 changed the behaviour of basemap.addcyclic. The data and longitude arrays are now being treated the same, i.e. the first point being appended to the end, but previously the longitude array was handled by adding the difference between longitudes to the last longitude, which is in my mind the correct behaviour.

An example:

import numpy as np
from mpl_toolkits.basemap import addcyclic

lons = np.arange(0, 360, 60)
data = np.ones([5, len(lons)]) * np.arange(len(lons))
new_data, new_lons = addcyclic(data, lons)
print new_lons

Prior to #138 being merged this would result in [0, 60, 120, 180, 240, 300, 360] but after it results in [0, 60, 120, 180, 240, 300, 0].

[jswhit]

You're right - this should be fixed.

[jswhit]

I've reverted this merge, so the old addcyclic has been restored.

[j08lue]

Maybe we can join forces here, Andrew? I saw that you recently added a similar function to cartopy.util (SciTools/cartopy#394) but that one lacks support for irregular grids and 2D arrays, too. Perhaps we can make a common new function for both packages?

A problem with your approach to add dlon to the duplicate last value/column is that it does not give the right result when the longitude spacing is irregular.

Also, if the last column already contains 360 or anything >360-dlon, the cyclic column will have values above 360 - but I guess these are handled well in both Cartopy and Basemap.

The question is: do the coordinate values have to be monotonically increasing? They are not, in the case of the grids I am having in mind (tri-polar), even without the added cyclic column. In Basemap, I sometimes get bad artifacts from that (the famous dateline problem, I guess, that I wish I was the guy to fix). I do not have as much experience with Cartopy yet, but at least with pcolormesh it seems to work fine with non-increasing longitudes.

In the function I proposed, I could easily make sure that the last two values of each row in lon are in increasing order to get the behavior you expect. However, that will not avoid problems with non-increasing values elsewhere. And for Cartopy, this does not seem to be necessary in the first place.

So where do we go from here? Would you consider making a function that works for irregular grids and 2D lon arrays based on your current Cartopy version, Andrew? As you did such nice work on that, perhaps you are the right one for the job. :-)

[ajdawson]

Well, ideally the cyclic longitude is represented as the first longitude + the modulo of the cyclic dimension (so 360). If you don't mind hard-coding the modulo then you can just add this to the cyclic column of the longitudes and you are done.

This assumes the coordinate increases monotonically in longitude along the cyclic dimension, but does not assume equal spacing along that dimension. You can extend this by checking the direction of the longitude dimension and subtracting the modulo instead of adding it if the coordinate is monotonically decreasing.

The question of whether you have to do this... I'd suspect you do, since it avoids having duplicate values for any given coordinate location. This means that your implementation will need to tell the difference between a data array and a longitude coordinate array, which unfortunately ruins the simplicity of your code. However, I am afraid this is just the way it has to be, data arrays and coordinates need to be treated differently at a fundamental level.

[WeatherGod]

Just wondering if the use of np.unwrap() would work here?

On Mon, Mar 10, 2014 at 10:19 AM, Andrew Dawson notifications@github.comwrote:

Well, ideally the cyclic longitude is represented as the first longitude +
the modulo of the cyclic dimension (so 360). If you don't mind hard-coding
the modulo then you can just add this to the cyclic column of the
longitudes and you are done.

This assumes the coordinate increases monotonically in longitude along the
cyclic dimension, but does not assume equal spacing along that dimension.
You can extend this by checking the direction of the longitude dimension
and subtracting the modulo instead of adding it if the coordinate is
monotonically decreasing.

The question of whether you have to do this... I'd suspect you do, since
it avoids having duplicate values for any given coordinate location. This
means that your implementation will need to tell the difference between a
data array and a longitude coordinate array, which unfortunately ruins the
simplicity of your code. However, I am afraid this is just the way it has
to be, data arrays and coordinates need to be treated differently at a
fundamental level.

Reply to this email directly or view it on GitHubhttps://github.com//issues/139#issuecomment-37186508
.

[ajdawson]

Just wondering if the use of np.unwrap() would work here?

It could be used I suppose but it doesn't really add much I don't think. We still need to treat the longitude and data arrays separately.

[jswhit]

I modified the calculation of the cyclic longitude point as suggested by ajdawson, so it no longer assumes the grid is regular (pull request #144).

[j08lue]

@jswhit I don't know if you are still interested, but I made another version of addcyclic that has the same functionality as my original suggestion (multiple input arrays besides the longitudes, axis keyword) plus incorporates the things we discussed (length of cyclic dimension as a parameter, maintains the increasing or decreasing order of coordinates). In case you are, please see PR #147 (tests can be added upon request). No worries if not.

[jswhit]

Sure, I'll have a look this weekend. Thanks.

[ajheaps]

This regression has appeared again in 1.0.8dev0 and 1.1.0 on conda-forge.
1.0.7: 1.875, 5.625, ..., 358.125 went to 1.875, 5.625,..., 358.125, 361.875
1.1.0: 1.875, 5.625, ..., 358.125 went to 1.875, 5.625,..., 358.125, 1.875

I can put a fix in my code if this is the new intended behaviour.