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

Typed Kriging #9

Merged
merged 3 commits into from
Jun 5, 2019
Merged

Typed Kriging #9

merged 3 commits into from
Jun 5, 2019

Conversation

ludoro
Copy link
Contributor

@ludoro ludoro commented Jun 2, 2019

Used constructor for type Kriging, refactored code. Not tests at the moment

struct Kriging <: AbstractBasisFunction
x
y
p
Copy link
Member

Choose a reason for hiding this comment

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

Do we need x y and p here? It looks to me this is being passed by the user?
One case I could think of is maybe for a plot recipe.

Copy link
Member

Choose a reason for hiding this comment

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

I see what you are doing. I think this struct should be split into 2, Kriging and a KrigingResult and then store the results in KrigingResult.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your input! The idea was to store everything in Kriging because later I might get into adaptivity (I get 3 more data points, how do I change the coefficients?) and I might need the "old" x's. Not sure If I explained myself correctly.
Do you think I should create a struct Kriging() that stores x,y,p,phi and q and then pass it to the constructor KrigingResult() which has the corresponding mu,b,...,result and std_error?

Copy link
Member

@Vaibhavdixit02 Vaibhavdixit02 Jun 3, 2019

Choose a reason for hiding this comment

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

I see, your approach seems correct then 👍

Copy link
Member

Choose a reason for hiding this comment

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

But you don't need the result pieces, right? You do k = Kriging(x::Array,y::Array,p::Number) to make it, and then do a call-overloading to do k(new_point) which returns a KrigingResult and adds the new points to the estimation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I am following what you are saying, but Kriging has x,y,p,mu,b.sigma,inverse_of_R,new_point,prediction and std_error as elements of the struct. The result pieces are needed for the construction of the type Kriging. So I am not sure how I could build the struct "Kriging" without also computing those elements?

@ludoro
Copy link
Contributor Author

ludoro commented Jun 5, 2019

Merging autonomously as discussed on Slack. I will do another PR for the tests of both Kriging and Radials to keep things tidy.

@ludoro ludoro merged commit 14efcdd into master Jun 5, 2019
@ludoro ludoro deleted the typed_kriging branch June 5, 2019 21:16
@ludoro ludoro mentioned this pull request Jun 6, 2019
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

Successfully merging this pull request may close these issues.

3 participants