ggobi / tourr

A implementation of tour algorithms in R
http://ggobi.github.io/tourr/
Other
65 stars 21 forks source link

interpolate function is broken #49

Closed uschiLaa closed 4 years ago

uschiLaa commented 4 years ago

Using the example code provided:

t1 <- save_history(flea[, 1:6], grand_tour(1), max = 10) Converting input data to the required matrix format. dim(t1) [1] 6 1 10 dim(interpolate(t1, 0.01)) [1] 6 1 2

Starting with 10 bases, interpolate returns only two bases, and looking at the bases shows that it is the initial basis repeated twice:

interpolate(t1) , , 1

        [,1]

[1,] 0.19811164 [2,] -0.53411260 [3,] 0.33825641 [4,] -0.68476359 [5,] -0.06762363 [6,] 0.29594589

, , 2

        [,1]

[1,] 0.19811164 [2,] -0.53411260 [3,] 0.33825641 [4,] -0.68476359 [5,] -0.06762363 [6,] 0.29594589

attr(,"new_basis") [1] TRUE TRUE attr(,"class") [1] "history_array" "array"

dicook commented 4 years ago

@huizezhang-sherry really need you to look at this one

for some reason tour(angle) is setting step$step to -1

dicook commented 4 years ago

Ok, got the problem spot - its in this code, in tour-planned.r:

  if (is.null(current)){
    return(basis_set[[1]])
  }

that was added in May. I think that there is no "current" in this function, so need to do this check a different way. Its necessary because something else breaks when I comment it out.

huizezhang-sherry commented 4 years ago

This can be a fix. Changing the starting value of index from 0 to 1 also makes it work.

On Wed, Aug 19, 2020 at 11:02 AM Dianne Cook notifications@github.com wrote:

Ok, got the problem spot - its in this code, in tour-planned.r:

if (is.null(current)){ return(basis_set[[1]]) }

that was added in May. I think that there is no "current" in this function, so need to do this check a different way. Its necessary because something else breaks when I comment it out.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ggobi/tourr/issues/49#issuecomment-675792410, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIYT3PXEN4KFQ3RIA5JKEADSBMQDHANCNFSM4QBGJZPQ .

-- Kindly Regards, Sherry Zhang

dicook commented 4 years ago

i don't think changing the index does anything. the problem is that

is.null(current) is TRUE, so the basis_set[[1]] is returned

dicook commented 4 years ago

Oh, yes it works. (I had ben using cycle = TRUE which is broken)

Won't this mean that the interpolation between first and second bases is dropped? Checking this now

dicook commented 4 years ago

starting and ending bases are the same - good!

and

table(attr(t1_interp, "new_basis"))

FALSE TRUE 220 10

good, same number of new_bases as given in t1

and

which(attr(t1_interp, "new_basis") == TRUE) [1] 1 23 51 79 108 136 158 184 213 230 t1[[2]] [,1] [1,] 0.23521165 [2,] -0.44955344 [3,] 0.67066819 [4,] 0.53059966 [5,] -0.01567487 [6,] -0.10487928 t1_interp[[23]] [,1] [1,] -0.23521165 [2,] 0.44955344 [3,] -0.67066819 [4,] -0.53059966 [5,] 0.01567487 [6,] 0.10487928

great, they match!

huizezhang-sherry commented 4 years ago

The first two are there:

t1 <- save_history(flea[, 1:6], grand_tour(1), max = 10)
t1[,,1:3]
a <- interpolate(t1)
a[,,attr(a, "new_basis")][,,1:3]
dicook commented 4 years ago

ok, checking code back in now

dicook commented 4 years ago

@uschiLaa can you check that this works for you?

uschiLaa commented 4 years ago

Thanks for the fix, it works for me as well!