natverse / nat.nblast

R package implementing the NBLAST neuron search algorithm, as an add-on for the NeuroAnatomy Toolbox (nat) R package.
http://natverse.org/nat.nblast/
17 stars 6 forks source link

Conversion to dotprops for nblast(neuron, dotprops) [or vice versa] is rather inefficient #25

Open jdmanton opened 10 years ago

jdmanton commented 10 years ago

As the conversion happens in WeightedNNBasedLinesetMatching(), this means that the objects are converted for every comparison, not just once at the start, making everything much slower than it needs to be.

jefferis commented 10 years ago

There is a difference between the conversion in WeightedNNBasedLinesetMatching and converting using e.g. dotprops(,resample=T). In the first case the direction vector is the vector from each point to its head point, without resampling. In the second case it is based on k nearest neighbours. It would probably be best to stick to pre-conversion to dotprops in most cases. I haven't profiled in ages, but I am not sure that the conversion part of WeightedNNBasedLinesetMatching.neuron is that slow since there is no nearest neighbour calculation or eigenvector decomposition.

jdmanton commented 10 years ago

I meant the conversion I added in d55c544 to allow it to handle the situation where query and target are not of the same class. I tried doing this higher up, in nblast(), but this messed a few things up — it looks like it is worth me retrying this and attempting to fix the problems.