search

NeuralEnsemble / python-neo

Neo is a package for representing electrophysiology data in Python, together with support for reading a wide range of neurophysiology file formats
http://neo.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
325 stars 248 forks source link

Fix pl2 channel enabled bug #1546

Closed nikhilchandra closed 2 months ago

nikhilchandra commented 2 months ago

Neo fails to load a PL2 file if there are channels within the source that are disabled for recording, though otherwise enabled to be visualized. This patch fixes that. See below for details.

PL2 files are recorded using the "PlexControl" application. When setting up an experiment, the researcher has access to a number of controls for each channel. Included among these are the following two options:

(1) Enabled -- this corresponds to the "m_ChannelEnabled" flag in each channel header (2) Record Enabled -- this corresponds to the "m_ChannelRecordingEnabled" flag in each channel header

The first option determines whether the channel's incoming data even makes it to the PlexControl application for visualization. The second option determines whether the incoming data for that channel is recorded to disk. The two options are decoupled in software, which means there are 4 possible scenarios:

  1. m_ChannelEnabled=True and m_ChannelRecordingEnabled=True --> Here, data for the channel makes it to PlexControl and is recorded to disk
  2. m_ChannelEnabled=True and m_ChannelRecordingEnabled=False --> Here, data for the channel makes it to PlexControl but is not recorded to disk
  3. m_ChannelEnabled=False and m_ChannelRecordingEnabled=True --> Here, data for the channel does not make it to PlexControl and so there is nothing to record to disk
  4. m_ChannelEnabled=False and m_ChannelRecordingEnabled=False --> Here, data for the channel does not make it to PlexControl and nothing is recorded to disk

Only Scenario 1 results in data being recorded to disk.

The code in plexon2rawio.py wants to consider only active channels, i.e., channels for which there is data. The original code defines active channels based solely on m_ChannelEnabled, which means that in Scenario 2 the code breaks when it looks for data that isn't there. The fix is to define active channels using both m_ChannelEnabled and m_ChannelRecordingEnabled.

zm711 commented 2 months ago

@h-mayorquin I think we should probably merge this one first and then we can merge yours after you add this into your PR. Let me run the tests and we will see tests first, but this makes sense to me. Thanks @nikhilchandra!

zm711 commented 2 months ago

@nikhilchandra,

Any idea why the plexon2 dll stochastically fails when trying to read files. We already had a discussion with Chris, but just curious if you've thought about it more at plexon.

h-mayorquin commented 2 months ago

@nikhilchandra,

Any idea why the plexon2 dll stochastically fails when trying to read files. We already had a discussion with Chris, but just curious if you've thought about it more at plexon.

As a context, it only fails in Ubuntu so I think is a wine or zugbruecke (I wrote the name of the facebook dude the first time) problem.

I agree that we should merge this before my PR.

zm711 commented 2 months ago

zugbruecke (I wrote the name of the facebook dude the first time) problem.

hahhahahah.

Okay, I'll merge, but if you have any ideas Nikhil feel free to post here or in Heberto's attached PR.

nikhilchandra commented 2 months ago

@zm711 Having never worked with Plexon DLLs in Ubuntu I don't really have an idea of what the problem may be, unfortunately.

@h-mayorquin I am working on putting together a test case for this, but it will require the creation of a specially-built .pl2 file. Is this okay?

h-mayorquin commented 2 months ago

That would be great. Is it possible for you to make the file smaller than 10 MiB?

zm711 commented 2 months ago

Thanks small test data would be awesome! Like Heberto said we aim for < 10 :)

Yeah I guess it could be a wine/zugbruecke. Maybe I can test some pl2 stuff on my Windows desktop and see if run a script a few times if I get an fails locally.

h-mayorquin commented 2 months ago

I have never seen a failure on the Windows or mac CI. I also think it was related to the 32-bit system as the Ubuntu interface uses the 32-bit DLC. It is a really though one that I wish I had way more time to figure out.

nikhilchandra commented 2 months ago

@h-mayorquin I have created a PL2 file and am figuring out how to set up the test. I see that your data files come from here:

https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/src/master/plexon

What is the procedure for adding a new file to this repo? Same as with github? Or is access restricted?

h-mayorquin commented 2 months ago

It is somewhat restricted. The easiest thing is for you to send the file to me and I can upload it.

h-mayorquin commented 2 months ago

I opened the PR here: https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/pulls/139

@zm711 can you give it a look?

zm711 commented 2 months ago

Thanks for the file @nikhilchandra! We have uploaded into the testing repo and now we can add it to the testing framework here :)

And of course thanks Heberto for getting them uploaded to gin.