Fix kcoords for non-int groups #3944
Conversation
| @@ -438,7 +438,9 @@ def _setup_json_probe_map(cls, recording, sorter_output_folder): | |||
| chanMap = np.arange(n_chan) | |||
| xc = positions[:, 0] | |||
| yc = positions[:, 1] | |||
| kcoords = groups.astype(float) | |||
| unique_groups = np.unique(groups) | |||
There was a problem hiding this comment.
Remember the bug with np.unique, we should use set.
There was a problem hiding this comment.
Right! Let me change that!
There was a problem hiding this comment.
What's the np.unique bug? We use it a lot!
There was a problem hiding this comment.
I vaguely remember a potential overflow issue since numpy is returning np.int* instead of Python int (of course in case the elements are ints). @h-mayorquin how wrong am I?
There was a problem hiding this comment.
Hi, guys, sorry I should have been more clear but I actually don't remember. At some point we encountered some bug with np.unique with mixed types and I have been weary of using since most of the times a set just does the same.
They are enhancing the np.unique so maybe the old bug is gone but I don't have time to track it:
| kcoords = groups.astype(float) | ||
| unique_groups = np.unique(groups) | ||
| group_map = {group: idx for idx, group in enumerate(unique_groups)} | ||
| kcoords = np.array([group_map[group] for group in groups], dtype=int) |
There was a problem hiding this comment.
I guess the other option here is
kcoords = np.array(group_map.values(), dtype=int)
This just makes it more readable for me, but doesn't help with the code so feel free to ignore.
There was a problem hiding this comment.
@zm711 just realized that doesn't work. We need kcoords to be the same length as x and y. Reverted
There was a problem hiding this comment.
Hello, is this np.array gonna fail when the groups have a different number of electrodes in each one? i.e. when you have some configuration with 182 electodes on shank 1, 96 on shank 2 and 96 on shank 3.
There was a problem hiding this comment.
No because it returns a flat list of inta from 0 to Ngroups-1
There was a problem hiding this comment.
my bad. I actually had the same thought as I was falling asleep last night and was going to post when I woke up!
Introduced by #3852, if the channel groups are not strings they cannot be cast to float. This makes a map to integers