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

Better interaction between Variogram() and Kriging() #63

Closed
redhog opened this issue Nov 27, 2020 · 5 comments · Fixed by #73
Closed

Better interaction between Variogram() and Kriging() #63

redhog opened this issue Nov 27, 2020 · 5 comments · Fixed by #73
Assignees
Milestone

Comments

@redhog
Copy link
Collaborator

redhog commented Nov 27, 2020

  • Make Variogram().describe() output all relevant parameters needed to
    • Recreate the Variogram given coordinates and values
    • Perform kriging
  • Make Kriging() not take a Variogram() instance as parameter, but the dictionary returned by describe(), plus coordinates and values.
@mmaelicke
Copy link
Owner

Thanks for opening.
What I would like (cause it would help in our data backend as well) is that the new Kriging instance takes the parameters like:

OrdinaryKriging(variogram=None, coordinates=None, values=None, model='spherical' [....])

Then with a quick check if the variogram is given, it would work just like it did so far. For your use-case, you could just grab the coordinates and values from somewhere else and pass the output of Variogram.describe() like:

# with V being an Variogram instance
OrdinaryKriging(coordinates=V.coordinates, values=V.values, **V.describe())

As a note: the describe already has the variogram parameters like sill and nugget and so on. What's missing is the callable model function, right? The Variogram.fitted_model returns a lambda function that is already parameterized, see:

code = """model = lambda x: func(x, %s)""" % \

While this can't be pickled (internally, it's a lambda), the model function itself (imported from skgstat.models) should be pickleable and can be used along with the parameters. Then, for kriging there would be no need to recreate the Variogram instance. With the values, coordinates, the model (python function) and the parameters for the model, we can make Kriging work, right?
Maybe we design the attributes of OrdinaryKriging with this in mind?
Then, describe would include what it has so far, all other attributes from Variogram.__init__ and the callable model function?

@redhog
Copy link
Collaborator Author

redhog commented Nov 27, 2020

Yeh, this looks good.

describe() is missing the distance metric too I think... But what I would like too, is for it to actually include all argument sent to Variogram.init() (except coordinates and values), so that you could do something like

v1 = Variogram(coordinates=c1, values=v1....)
v2 = Variogram(coordinates=c2, values=v2, **v1.describe()["args"])

This is mostly for completeness.

@mmaelicke
Copy link
Owner

@redhog Do you want to assign yourself to this issue, or should I implement it?
For me, the #65 stuff has priority and needs to kind of work until mid next week, but if necessary, I could try to implement the new describe function on the weekend.

@mmaelicke mmaelicke added this to the Version 0.4 milestone Dec 1, 2020
@redhog redhog self-assigned this Dec 1, 2020
@redhog
Copy link
Collaborator Author

redhog commented Dec 1, 2020

Assigned it to me, but: I need to concentrate on #58 since this stuff works well enough for us as is (hackety version) right now, but we do have problems with performance on large datasets...

@mmaelicke
Copy link
Owner

yeah, NP. Just wanted to make sure, that you are not waiting for me...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants