Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird behaviour in bilinear_interp.py #57

Open
tommy307507 opened this issue Jun 7, 2021 · 3 comments
Open

Weird behaviour in bilinear_interp.py #57

tommy307507 opened this issue Jun 7, 2021 · 3 comments

Comments

@tommy307507
Copy link

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
Copy link
Author

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
Copy link
Author

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

@tsutterley
Copy link
Owner

tsutterley commented Jun 10, 2021

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 added a commit that referenced this issue Jun 13, 2021
feat: added 3413 projection for new 1km Greenland model from ESR

fix: tidal currents for bilinear interpolation to address part of #57

fix: add warning for tide models being entered as string to address #58

docs: update documentation for new model
tsutterley added a commit that referenced this issue Jun 15, 2021
feat: added 3413 projection for new 1km Greenland model from ESR

feat: update notebooks for Gr1km-v2

fix: tidal currents for bilinear interpolation to address part of #57

fix: add warning for tide models being entered as string to address #58

docs: update documentation for new model

docs: add notes to interpolation with warnings

fix: check for nan points when reading OTIS models

test: update list of invalid stations in Arctic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants