Skip to content

Conversation

nikhilchandra
Copy link

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.

Nikhil Chandra added 2 commits September 2, 2024 16:41
…orrectly handle PL2 (Plexon2) files with enabled but unrecorded channels.
…nnelRecordingEnabled (either one or the other does not work) for PL2 (Plexon2) files.
@zm711
Copy link
Contributor

zm711 commented Sep 4, 2024

@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 zm711 mentioned this pull request Sep 4, 2024
@zm711
Copy link
Contributor

zm711 commented Sep 4, 2024

@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.

Copy link
Contributor

@h-mayorquin h-mayorquin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks for writing such a clear explanation in your PR.

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

This is the really confusing case!

@h-mayorquin
Copy link
Contributor

@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
Copy link
Contributor

zm711 commented Sep 4, 2024

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.

@zm711 zm711 merged commit 0aa596e into NeuralEnsemble:master Sep 4, 2024
3 checks passed
@nikhilchandra
Copy link
Author

@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
Copy link
Contributor

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

@zm711
Copy link
Contributor

zm711 commented Sep 4, 2024

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
Copy link
Contributor

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.

@zm711 zm711 added this to the 0.14.0 milestone Sep 4, 2024
@nikhilchandra nikhilchandra deleted the fix_pl2_channel_enabled_bug branch September 4, 2024 22:15
@nikhilchandra
Copy link
Author

@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
Copy link
Contributor

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

@h-mayorquin
Copy link
Contributor

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

@zm711 can you give it a look?

@zm711
Copy link
Contributor

zm711 commented Sep 5, 2024

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.

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