pyTMD icon indicating copy to clipboard operation
pyTMD copied to clipboard

Weird behaviour in bilinear_interp.py

Open tommy307507 opened this issue 3 years ago • 3 comments

Issue one: np.nonzero returns a tuple of 2 np.array to represent the indices. in line 63 of bilinear_interp.py, there is an extra , behind the assignment. This causes the output of the tuple to be unpacked into 2 variables, one of them being always empty. As each "row" of the indices are the length of the flattened array, as shown in the usage in line 75,76 it can be used to extract from tuple elegantly, however at this line this causes the following Error: ValueError: operands could not be broadcast together with shapes ( lon , ) ( ilon * ilat , ilat)

Proposed "Fix" : Removing the , makes the function execute pass this line

Issue two: in line 66 to 68 , the array data is created using the shape npts, which when applied to a 2d numpy matrix, only shows part of its shape. e.g. 1000 x 500 matrix would have a len() of 1000 only, this then would produce a 1d array used to store data. This would then cause another error of non-broadcastable shapes: e.g. ValueError: could not broadcast input array from shape ( x , y ) into shape ( x ,)

tommy307507 avatar Jun 07 '21 08:06 tommy307507

I would want to add in read_tide_model.py there's also this issue of len(npts) being used incorrectly. The fix would be changing npts = len(D) to npts = D.shape , and adding an asterisk (*) to before the npts used in the assignment of placeholder matrices. i.e. np.ma.zeros((*npts,nc)....) and np.ma.zeros((*npts),...) below to use the starred expression. Thanks for the package but I had a hard time figuring this out …

tommy307507 avatar Jun 07 '21 08:06 tommy307507

In line 384 of read_tide_model.py, there is a missing [:] after v1.data causing the error: AttributeError: can't set attribute v1.data = bilinear_interp(xi,yv,v,x,y,dtype=np.complex128)

proposed fix: adding the [:], this also matches the syntax used in line 324 for u1.data

tommy307507 avatar Jun 08 '21 06:06 tommy307507

Hi @tommy307507, sorry I'm out of town so I'm a little slow on responses.

These are good points. I typically flatten the input fields so I don't run into the shape issues. It's a good point that I should add some logic for different shaped fields for people not using the "higher order" programs for computing heights or currents. I should also update the documentation so that people don't run into the problems you've noted.

I'll definitely fix the attribute error as soon as I get back in the office. Or if you want to put a PR in to fix it that would be great too.

tsutterley avatar Jun 10 '21 02:06 tsutterley