-
Notifications
You must be signed in to change notification settings - Fork 5
Add option to order Neuropixels channels by depth #513
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
Conversation
jonnew
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
|
The main challenge here is mapping between the channel index (i.e., The initial idea (issue comment) was to use a dictionary with the channel as the key and its row/column as the value. I explored alternatives but couldn’t avoid this without pushing the complexity to O(N²), since we need to retrieve the row/column from a given channel index to remap for spatial ordering. We could do this in one pass, but it would require four nested loops (row/col iteration + searching for the matching channel), which might not be worth the tradeoff. The current approach uses O(N) time for this mapping step and O(N log N) overall due to sorting (ref). The dictionary increases space complexity, but it avoids the nested search. I also removed a redundant remapping operation and verified the algorithm on a 2×3 array by hand, which matched the expected behavior (testing with the |
|
This seems ok to me, seems a good approach. But I'd prefer having @jonnew's confirmation since I'm not that familiar with neuropixel stuff. |
|
Due to merge conflicts, it might be easiest to wait until #536 is merged, and then resolve conflicts here. Another point to watch out for; if #523 is merged before this is, update the
ETA: Merge conflicts have been fixed, as both PRs were merged at the same time. |
This PR adds the ability for data nodes to
OrderByDepth, so that instead of following the conventional channel ordering (0 -> 383), the channels are instead ordered by their physical location along the shank. Therefore, if channel 0 corresponds to electrode 384 (Bank B), but all other channels are on Bank A (1 -> 383), then the channels will be ordered so that channel 0 is electrode 1 (normally channel 1), channel 1 is electrode 2 (normally channel 2), and so on until the very last channel is electrode 384 (normally channel 0).This option is added as a boolean to all data nodes, and is
falseby default, so that if the user loads an old workflow but doesn't change anything, their channel map remains unchanged.The algorithm logic is first proposed in the issue itself, and has been updated to reflect the current logic in the code.
Fixes #440