Metric space

[redhog]

Continuation of #68 but within this repo

[mmaelicke]

@redhog are you currently working on more commits?

[redhog]

Nope :)

[mmaelicke]

@redhog, from the conversation I get that I will turn a few assert statements into an if today. What about the VariogramResult? you mentioned it could be dropped...
Can I go forward and do that to finally merge today, or is there something that we need to take care of / discuss?

Finally, just for my understanding, a reply to #84 (comment) would be great, so that I understand your code as well :)

Anything you would like to add?

Thank you man, I am really looking forward to use these nice code extensions.

[redhog]

Yes, just drop VariogramResult.
And no, I think any other fixes/changes I make would have to go to their own PRs - this is getting way too big...

[mmaelicke]

@redhog I am finished with my changes. But I have broken something. All 7 tests fail at the same line:

scikit-gstat/skgstat/MetricSpace.py

Line 49 in d6b05e2

ridx = np.where(dists <= max_dist)[0]

I don't get it right now. Maybe I am in front of my computer for a few hours too long today.
Did I unindent something? Can you help me out?

[mmaelicke]

@redhog ,
Unfortunately, I found a serious issue with this branch. I ran a notebook this morning producing some figures for my next publication. With the metric-space branch still active, a new variograms changed their shapes dramatically, without chaning the code. After some intensive code review, I think there is something wrong with the maxlag / max_dist settings. I tried to put an example together.
Consider the two examples below. I am only setting a maxlag, that should not change the variogram, as I am using the largest bin as a maxlag. For the master branch everything is fine and the two variograms look the same:

For the metric-space branch the two variograms differ dramatically. Note: the coordinates is a ndarray of shape (500,2).

I am right now not sure, what is happening and you might have the better insight into the code. Maybe the grouping into lag classes is somehow not prepared for some kind of edge case. Other variograms in the very same notebook do not show this behavior.

[mmaelicke]

@redhog: Edit: here is the sample dataset if you want to run / try it yourself:
sample.csv

[redhog]

Thanks, was gonna ask for the sample dataset :) Will try it out and try to figure out what's boken ASAP....

[mmaelicke]

@redhog Thanks, appreciate it!
I just pushed commit 0c617c5 to master, which includes the test data and a test that reproduces the example. You can also use that one to test a bugfix. (And this would also be another good place to put more tests that need actual spatial correlated data in the future...)

[redhog]

There was some strangeness in the handling of max_dists (self.max_dist overriding max_dist to find_closest). With that fix, your new test, cherry-picked from master, passes.

[mmaelicke]

I'll merge and release in a minute.
Thanks @redhog, this is an awesome PR that really makes the lib better.

[redhog]

:) So happy we managed to finally push it all the way!