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

Inherit kd_tree resampler for bilinear interpolation #166

Closed
wants to merge 10 commits into from

Conversation

pnuu
Copy link
Member

@pnuu pnuu commented Mar 6, 2019

This PR updates bilinear interpolation so that it inherits XArrayResamplerNN as a base.

  • Closes #xxxx
  • Tests added
  • Tests passed
  • Passes git diff origin/master **/*py | flake8 --diff
  • Fully documented

da = None
from xarray import DataArray
import dask.array as da
import dask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

F401 'dask' imported but unused

vii_slices=vii_slices, ia_slices=ia_slices,
fill_value=fill_value,
dtype=new_data.dtype, concatenate=True)
#new_axes={'neighbour_dim': self.neighbours})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E114 indentation is not a multiple of four (comment)
E116 unexpected indentation (comment)

vii_slices=vii_slices, ia_slices=ia_slices,
fill_value=fill_value,
dtype=new_data.dtype, concatenate=True)
# new_axes={'neighbour_dim': self.neighbours})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E116 unexpected indentation (comment)

@pnuu
Copy link
Member Author

pnuu commented Mar 6, 2019

So far this PR has only made the bilinear interpolation much slower. With the script below, it takes 8 m 40 s for the first run and 4 m 7 s for the second run on my laptop. The same figures for master branch are 4 m 40 s / 2 m 42 s.

Performance for nearest hasn't changed with the small changes that were needed.

#!/usr/bin/env python

import glob                                                            
from satpy import Scene                                                
fnames = glob.glob('/home/lahtinep/data/satellite/geo/msg/H*')
glbl = Scene(reader='seviri_l1b_hrit', filenames=fnames)                
glbl.load(['overview', 'natural_color', 'realistic_colors'])  
lcl = glbl.resample('euro4', resampler='bilinear', cache_dir='/tmp')
lcl.save_datasets(base_dir='/tmp')

@@ -926,11 +930,11 @@ def query_no_distance(target_lons, target_lats, valid_output_index,
if index_array.ndim == 1:
index_array = index_array[:, None]

# KDTree query returns out-of-bounds neighbors as `len(arr)`
# KDTree query returns out-of-bounds neighbours as `len(arr)`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pnuu We are trying to stick with US English spelling of things now. Your change makes this the UK English. Please undo these changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will break backwards compatibility, neighbours is used in the method and function names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but not sure comments needed to be changed. I'd be willing to deprecate the old neighbours methods over time (like until a 2.0 release).

@pnuu pnuu added obsolete This is obsolete due to other changes and removed in progress labels Sep 19, 2019
@pnuu
Copy link
Member Author

pnuu commented Sep 19, 2019

I'm closing this as obsolete. All the other changes made after this was created make it hard to directly update. I might revisit this in a new PR started from more up-to-date starting point.

@pnuu pnuu closed this Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted obsolete This is obsolete due to other changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants