cKDTree optimization #62
Conversation
redhog
force-pushed
the
ckdtree-optimization
branch
from
6af812c to
9a9466c
Compare
skgstat/Kriging.py
Outdated
| @@ -389,16 +395,23 @@ def _krige(self, idx): | |||
| dists = self.transform_dists[idx,:] | |||
|
|
|||
| # find all points within the search distance | |||
| idx = np.where(dists <= self.range)[0] | |||
| if isinstance(dists, scipy.sparse.spmatrix): | |||
| idx = np.array([k[1] for k in dists.keys()]) | |||
There was a problem hiding this comment.
For non-sparse datasets this might actually be a performance bottleneck, and we should use toarray() or somesuch solution to speed it up.
|
Heyas @mmaelicke ! Would you have time to review this? Note: There is a new flag "sparse" added. If set to false, all distances are calculated using pdist, which is the fastest way, at least for semi-small datasets, but quickly eats a lot of ram (N*M, where N is number of points kriged to and M is number of points kriged from). If set to true, distances are calculated using ckDTree only for points withing range, and stored in a sparse matrix. This takes considerably less storage, but it slightly slower for lookup, so for smaller datasets this is a disadvantage. For larger datasets this saves your machine from swapping, which would quickly lower your performance. |
|
@redhog , awesome, thank a lot! |
|
Maybe the default for sparse needs to be False... Hm... |
| selected_dists = dists[0, idx].toarray()[0,:] | ||
| else: | ||
| selected_dists = dists[idx] | ||
| sorted_idx = np.argsort(selected_dists, kind="stable") |
There was a problem hiding this comment.
just out of curiosity: why stable sort here?
There was a problem hiding this comment.
Because without that I couldn't do a regression test that gave the same result with sparse/non-sparse (when point coords coincided exactly for some pair of points, which some did in my test dataset)..
|
This is superseded by #68 which implements the same feature, but in a separate class that can also be used by Variogram. |
|
Only merge the other one. Sorry for coding faster than I talked to you... |
|
I had the other one in parallel, but didn't want to push it due to a bug that I have now squished... |
|
NP. I will just wait until you want me to review or merge something. Or both. :) |
Closes #58