rotate_vector major bug in basemap

Jeff,

I ran into a major bug that I think results from a fairly deep-seated gotcha in basemap. The problem lies with the in-place transformation of lon,lat in the proj library. This is not an inherent problem in itself; the trouble comes because sometimes the data get copied along the way from the basemap interface into proj, and sometimes they don't. When they don't get copied, rotate_vector breaks, because the input lon,lat variables are transformed in place, and then they are no longer lon and lat when they need to be reused as such.

This could be fixed with a copy in rotate_vector, but I recommend that instead it be fixed by guaranteeing that there is a copy at a much lower level, probably in the pyproj Proj.__call__ method, and if not there, then in the basemap.__call__ method. I am not sure what all the effects might be, though--is anything relying on in-place calculation at that level?

The reason for recommending the low-level copy is that if basemap.__call__ continues to behave as it does, sometimes obliterating its input arguments via the in-place operation and sometimes not, it will continue to cause horrible surprises. I think that the overhead of a single copy would be a small price to pay to eliminate such surprises.

If there is a need to preserve the in-place option, then perhaps it could be accessed through a different interface. Let __call__ guarantee that its input arguments are preserved, and have a "transform" method that minimizes copying.

Eric

Jeff,

I sent that last message a little too quickly. Now I see that pyproj really is trying to ensure a copy--that is what the comments and docstring say--so I will simply try to find and fix the bug, the particular case in which a copy is not being made.

Eric

Eric Firing wrote:

···

Jeff,

I ran into a major bug that I think results from a fairly deep-seated gotcha in basemap. The problem lies with the in-place transformation of lon,lat in the proj library. This is not an inherent problem in itself; the trouble comes because sometimes the data get copied along the way from the basemap interface into proj, and sometimes they don't. When they don't get copied, rotate_vector breaks, because the input lon,lat variables are transformed in place, and then they are no longer lon and lat when they need to be reused as such.

This could be fixed with a copy in rotate_vector, but I recommend that instead it be fixed by guaranteeing that there is a copy at a much lower level, probably in the pyproj Proj.__call__ method, and if not there, then in the basemap.__call__ method. I am not sure what all the effects might be, though--is anything relying on in-place calculation at that level?

The reason for recommending the low-level copy is that if basemap.__call__ continues to behave as it does, sometimes obliterating its input arguments via the in-place operation and sometimes not, it will continue to cause horrible surprises. I think that the overhead of a single copy would be a small price to pay to eliminate such surprises.

If there is a need to preserve the in-place option, then perhaps it could be accessed through a different interface. Let __call__ guarantee that its input arguments are preserved, and have a "transform" method that minimizes copying.

Eric

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Jeff,

The problem is not in basemap, it is numpy. Basemap is doing the right thing, using the astype method to ensure a copy--but that is not what the method is actually doing in this case. I filed a numpy ticket (788). I took a quick look at the numpy code but did not find anything obviously amiss.

If this does not get solved in numpy right away, the workaround in basemap/pyproj _copytobuffer is to use npy.array(x, dtype=npy.float64, copy=True) instead of the astype method.

Eric

Eric Firing wrote:

Jeff,

I sent that last message a little too quickly. Now I see that pyproj really is trying to ensure a copy--that is what the comments and docstring say--so I will simply try to find and fix the bug, the particular case in which a copy is not being made.

Eric

Eric Firing wrote:

Jeff,

I ran into a major bug that I think results from a fairly deep-seated gotcha in basemap. The problem lies with the in-place transformation of

[...]

Eric Firing wrote:

Jeff,

The problem is not in basemap, it is numpy. Basemap is doing the right thing, using the astype method to ensure a copy--but that is not what the method is actually doing in this case. I filed a numpy ticket (788). I took a quick look at the numpy code but did not find anything obviously amiss.

If this does not get solved in numpy right away, the workaround in basemap/pyproj _copytobuffer is to use npy.array(x, dtype=npy.float64, copy=True) instead of the astype method.

Eric

Eric: Thanks a lot for tracking this down - great detective work! I'll keep an eye on the ticket and if it doesn't get fixed in 1.1 I'll use your suggested workaround.

-Jeff

···

Eric Firing wrote:

Jeff,

I sent that last message a little too quickly. Now I see that pyproj really is trying to ensure a copy--that is what the comments and docstring say--so I will simply try to find and fix the bug, the particular case in which a copy is not being made.

Eric

Eric Firing wrote:

Jeff,

I ran into a major bug that I think results from a fairly deep-seated gotcha in basemap. The problem lies with the in-place transformation of

[...]

--
Jeffrey S. Whitaker Phone : (303)497-6313
NOAA/OAR/CDC R/PSD1 FAX : (303)497-6449
325 Broadway Boulder, CO, USA 80305-3328

Jeff,

Travis fixed the problem on Saturday (r5155). Trac doesn't seem to have the nice feature of sending an email to the originator of a ticket when it is changed or closed, and I didn't check until this morning. I don't know whether this fix, or any other recent fixes, or fixes yet to be made (addressing memory leaks that seem to have cropped up--see ticket 774) will get into the release.

The problem that I reported was triggered by an array created by scipy.io.loadmat, reading an old-format mat file created by custom code. I suspect most users would never trip over it. I never did figure out what was different about the dtype of that array.

Eric

Jeff Whitaker wrote:

···

Eric Firing wrote:

Jeff,

The problem is not in basemap, it is numpy. Basemap is doing the right thing, using the astype method to ensure a copy--but that is not what the method is actually doing in this case. I filed a numpy ticket (788). I took a quick look at the numpy code but did not find anything obviously amiss.

If this does not get solved in numpy right away, the workaround in basemap/pyproj _copytobuffer is to use npy.array(x, dtype=npy.float64, copy=True) instead of the astype method.

Eric

Eric: Thanks a lot for tracking this down - great detective work! I'll keep an eye on the ticket and if it doesn't get fixed in 1.1 I'll use your suggested workaround.

-Jeff

Eric Firing wrote:

Jeff,

I sent that last message a little too quickly. Now I see that pyproj really is trying to ensure a copy--that is what the comments and docstring say--so I will simply try to find and fix the bug, the particular case in which a copy is not being made.

Eric

Eric Firing wrote:

Jeff,

I ran into a major bug that I think results from a fairly deep-seated gotcha in basemap. The problem lies with the in-place transformation of

[...]