theislab / destiny

R package for single cell and other data analysis using diffusion maps
https://theislab.github.io/destiny/
GNU General Public License v3.0
69 stars 12 forks source link

DPT() ignores tips=... #30

Closed MikeDMorgan closed 4 years ago

MikeDMorgan commented 4 years ago

The current implementation of the DPT() function appears to ignore the user values of the tips argument. If I pass either the root cell, or even a randomly sampled index, to DPT(dm, tips=root), then I always receive exactly the same DPT ordering in the DPT slot of the returned object.

Having re-implemented the DPT code externally I can generate the "correct" ordering, therefore, I can only assume that somewhere the tips argument is overiden. This issue arises with destiny 3.0.0 (R 3.6.1, Bioc 3.10) as well as the previous Bioc 3.8 version in R 3.5.3.

flying-sheep commented 4 years ago

Hmm, I didn’t really change anything in DPT in the current master version of destiny, but I can’t seem to reproduce this with it. Can you try it?

> packageVersion('destiny')
[1] ‘3.1.0’
> library(destiny)
> data(guo)
> dm <- DiffusionMap(guo)
> dpt <- DPT(dm)
> set.seed(4)
> dpt_random <- DPT(dm, tips = sample(ncol(guo), 3L))
> gridExtra::grid.arrange(plot(dpt), plot(dpt_random), ncol = 2)

grafik

MikeDMorgan commented 4 years ago

Thanks, it appears to be an issue with values stored specifically in the $DPT1 slot of the DPT object.

data(guo)
dm <- DiffusionMap(guo)
dpt <- DPT(dm)
set.seed(4)

dpt_random <- DPT(dm, tips = sample(ncol(guo), 3L))

guo.dm <- data.frame("DC1"=dm$DC1, "DC2"=dm$DC2, "DPT.Rand"=dpt_random$DPT1, "DPT.Set"=dpt$DPT1)

rand.dpt.p <- ggplot(guo.dm, aes(x=DC1, y=DC2, fill=DPT.Rand)) +
  geom_point(shape=21) +
  labs(title="Random tip") +
  scale_fill_viridis()

set.dpt.p <- ggplot(guo.dm, aes(x=DC1, y=DC2, fill=DPT.Set)) +
  geom_point(shape=21) +
  labs(title="Set tip") +
  scale_fill_viridis()

plot_grid(rand.dpt.p, set.dpt.p)

guo_dpt_tips

flying-sheep commented 4 years ago

I see! It’s a misconception, DPTx doesn’t mean “tip x” but “cell x”, see the implementation for $.DPT and [[.DPT:

https://github.com/theislab/destiny/blob/5d853e3cdbe8a821912348357f8e57b669fa50d9/R/methods-extraction.r#L57-L67

I need to document this better. What you want is

tips <- random_root(dm)  # or c(cell_id1, cell_id2, …)
dpt <- DPT(dm, tips)
pt_vec <- dpt[tips[[1]], ]

You can also do pt_mat <- dpt[tips, ] or pt_mat2 <- dpt[, tips], as you wish.

MikeDMorgan commented 4 years ago

Thanks @flying-sheep - this definitely needs to be documented clearly!

flying-sheep commented 4 years ago

Fixed in 33422f35210e75e4bb7b5cb486e80ef6c97e36e3. If you have any tips on how to make everything more discoverable, please tell me!