-
Notifications
You must be signed in to change notification settings - Fork 6
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
add plot3d.nblastres function and give nblast option to return per segment scores #29
Comments
Plot-wise, what differences from the |
I think I was imagining that this would be doing what show_similarity is now doing. What do you see as the distinct use cases? I did recently make a simple wrapper function for nblast searches again fly circuit data where there were then plot and summary methods for the results object. This was quite useful but not sure if we can make a general version. Will try to find code. G. Sent from my iPhone
|
@ajdm I think it would be good to add an example based on the kcs20 dataset to show_similarity and then to consider this issue closes at least for the purposes of the paper. |
Ah, I wrote a response to the 'use case' question, but it looks like I never pressed the comment button. I thought that the main use cases were:
I'll add an example for |
Kcs20 gamma (unbranched) vs ab will sort of do 1 & 4. G. Sent from my iPhone
|
An example (perhaps not a particularly great one) using gamma and alpha-beta neurons added in 917a6e8. |
I'm getting this: library(nat.nblast)
>
> library(nat)
>
> # Pull out gamma and alpha-beta neurons
> gamma_neurons <- kcs20[kcs20[,]$type=='gamma']
> ab_neurons <- kcs20[kcs20[,]$type=='ab']
>
> clear3d()
> show_similarity(ab_neurons[[1]], ab_neurons[[2]])
Show Traceback
Rerun with Debug
Error in get(getOption("nat.nblast.defaultsmat")) :
invalid first argument because |
Ah, I didn't add the change where I uncommented L5 in |
Done in 6b2e032. I can easily reverse the commit if it should do something else instead. |
I think that is fine, although maybe we should just make like easier for ourselves and set a default smat as you did on testing. More importantly though are you sure the colours are working as expected. When I do this: # nb I think these next 2 lines may be better style for interactive use than the current version
gamma_neurons <- subset(kcs20, type=='gamma')
ab_neurons <- subset(kcs20, type=='ab')
clear3d()
show_similarity(ab_neurons[[1]], gamma_neurons[[3]]) I get this Although the calyx looks red, which I assume to be bad, the dorsal tip of the alpha lobe doesn't look that bad but seems an equally long way from anything to me. |
I did wonder about the colours some times, but guessed that it was my intuition that was wrong. Your example does look pretty odd though, so I reckon something is indeed wrong. I'll investigate. |
The problem is that target and query are swapped here. The colours that you are getting are for the segments in the query neuron but you are plotting them for the target neuron. It is confusing. If you swap n1 and n2 here: f(PlotVectors) {
# We need to duplicate each colour as we are drawing line segments, not points
segcols <- c(sapply(segcols, function(x) c(x,x)))
plot3d(n2, col=col, PlotVectors=TRUE, PlotPoints=FALSE, ...)
plot3d(n1, col=segcols, PlotVectors=TRUE, PlotPoints=FALSE, ...)
} else {
plot3d(n2, col=col, PlotVectors=FALSE, PlotPoints=TRUE, ...)
plot3d(n1, col=segcols, PlotVectors=FALSE, PlotPoints=TRUE, ...)
} it should work. |
Ah, that would explain why it looked better for more similar neurons... Sorry. Fixed in 5fdc16f. |
Cool. But do the docs need updating to clarify who will be coloured etc? And is it the comparison that makes most sense? I haven't thought it through completely myself. |
Is it worth renaming |
Yes, I think that would be good. |
note also that the order of target and query args is different for |
Yeah, I just noticed that... I've updated the docs for |
I've swapped the arguments in 8025265. Now we compare the query to the target (and not the target to the query), as with |
I think swapping the args is best so +1 for that. Deciding which neuron should be coloured is more complicated. I think many people might expect the target to be coloured (because it is the hit, the thing that is being returned by the search). But in fact the per segment scores are really only defined for the query neuron. We could maybe give the option to colour either neuron, but that would involve mapping the scores for query segments onto the corresponding target segments. So let's go with what you have now. |
Should I submit this updated version to CRAN once I've fixed #31 too? |
Yes. But maybe wait a couple of days in case anything else surfaces. |
happy to close this issue now. |
In order to satisfy one of the reviewer comments we should add a new function / example that shows which points are being matched for a pair of neurons and colours one of the neurons by the quality of the match. One way to do this would be allow the
nblast
function to return per segment scores (perhaps wrapping them in an object with a class likenblastres
. A corresponding plot3d method could then be used to make a plot with sensible defaults.Alternatively, a lower tech version would be to include an example in the
nblast
docs.Collecting per segment results could be done by playing with the
NNDistFun
argument (which gets passed down toWeightedNNBasedLinesetMatching.default
.The text was updated successfully, but these errors were encountered: