Skip to content

Updated/Fixed code for MaxWell systems. #354

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

cmccrimmon
Copy link

Updated MaxWell array geometry to newer version and removed non-bijective channel-electrode mapping. The latter addresses the problem noted in NeuralEnsemble/python-neo#1703. Updated to avoid pandas usage compared to closed pull request #353.

cmccrimmon and others added 11 commits June 26, 2025 18:40
Updated to newer MaxOne electrode geometry.
Updated with new MaxOne electrode geometry.
Updated MaxWell array geometry to newer version and removed non-bijective channel-electrode mapping.
@samuelgarcia
Copy link
Member

OK for me.
Maybe a small comments inside the code to explain a bit more the non bijective story would be perfect to remember theses details.

@samuelgarcia
Copy link
Member

Thanks a lot for this update.
I would be happy to understand clearly the situation. in this maxwell format and the semantic.

If I understand correctly:

  • maxwell electrode refer to probeinterface contact (=on physical electrode contact)
  • maxwell channel refer to spikeinterface channel (=one signal trace)
  • maxwell, for some format version, is multiplexinag sevral contact on the same channel

If my anderstanding is correct, then here we are virtually removing the contacingt that are multiplexed and keep only the first. I understand the need with spikeinterface but from probeintefrace point-of-view (having information of the probe description) I am not sure this is a good idea to do it silently.

So I would probose to have an option like (remove_contact_with_multiplexed_channel=False) to control this behavior with a very detailed documention in th docstring.
Then on spikeinterface side we would do remove_contact_with_multiplexed_channel=True.

Maybe I am totally miunderstanding the situation.

@alejoe91
Copy link
Member

alejoe91 commented Jul 2, 2025

@samuelgarcia the current situation is nicely summed up by @philipp-mxw here: NeuralEnsemble/python-neo#1703 (comment)

Basically there is no multiplexing, but there are cases in which the same channel is connected to multiple electrodes.

So I think that the implementation here is correct, since it "finds" all electrodes that are used in a certain configuration.

@cmccrimmon tests fail because of the updated contact geometry. Can you fix the tests and use the new geometry?

@philipp-mxw could you double check this? :)

@samuelgarcia
Copy link
Member

Thank you @philipp-mxw for summary.

But we have still an issue then.
I I use the same example provide by philipp

Channel 399 -> electrodes [25732 13364]
Channel 431 -> electrodes [25732 13364]

Here, in the actual implementation we will keep only the first contact, which is weird and we loose the full mapping info.

Channel 399 -> electrodes 25732
Channel 431 -> electrodes 25732

I think having a clear flag that remove or not duplicated channels/electrode would help the maintenance and readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants