search

katzfuss-group / GPvecchia

Fast Gaussian-process inference using Vecchia approximations
19 stars 6 forks source link

provide matrix to vecchia_likelihood #76

Open jingjiezh opened 4 years ago

jingjiezh commented 4 years ago

The vecchia_likelihood function allows users to provide a matrix as 'covmodel'. However, this matrix (in the original ordering) can't be used directly. We must reorder the matrix based on the ordering provided to vecchia_specify.

So on help file for vecchia_likelihood when providing matrix: explain, and/or reorder internally based on ordering provided to vecchia_specify.

marcinjurek commented 4 years ago

Ok I looked into it and so the situation is a little more complicated. Generally speaking, we never envisioned passing the full covariance matrix to, say, vecchia_likelihood. There are three possible types of covmodel:

  1. a string ("matern" or "esqe")
  2. an R function
  3. an n by (m+1) matrix. The code checks the type of the argument (string, function, matrix) and applies the appropriate function. Thus the only supported case when covmodel can be a matrix, is when it is a matrix with the subset of covariance elements analogous to NNarray: the k-th row contains the covariances between the k-th gridpoint and those points whose indices can be found in the k-th row of the NNarray. I hope this makes sense.

If one really wants to start with the covariance matrix, like in the case of filtering, another function, getMatCov can be used to extract this reduced matrix from the full matrix. In fact, this is a crucial function in all filtering applications of GPvecchia, like the ones I worked on.

katzfuss commented 4 years ago

@marcinjurek Can you please add this explanation to the help file? Thank you!

katzfuss commented 4 years ago

Also, I believe that passing an nxn matrix does work, as long as it is ordered appropriately (as Jingjie said). Of course, this is not actually scalable to large n. I think your explanations and options for types 2 and 3 are only applicable in the MRA case.

marcinjurek commented 4 years ago

ok I just pushed a proposed fix. It is on a separate branch so that the main repo looks unchanged. @jingjiezh, could you please pull this branch (fixing76), test if it does what you need and let me know? If so, I will merge it to master (or feel free to merge it yourself if it works)

jingjiezh commented 4 years ago

Thank you @marcinjurek. I looked at the changes to createU, and now the covmodel looks good. I also tested and get the likelihood as expected.