catalystneuro / mease-lab-to-nwb

MIT License
3 stars 2 forks source link

Different number of rising/falling edges #47

Closed ross-folkard closed 2 years ago

ross-folkard commented 3 years ago

Hi there, I'm trying to add some TTL traces from CED into and existing nwb file with a notebook from I think @CodyCBakerPhD . I run into the following issue.

image image

When ignoring this line, the code runs, but I'm wondering how vital this line is, and whether the resulting file will make sense without it.

Any help would be much appreciated!

All the best

Ross

alejoe91 commented 3 years ago

Hi @rf13734

So that means that the acquisition was stopped before the TTL falling phase. I agree this is probably not an issue. I can push a fix excluding TTLs that are not complete. Would that be ok for you?

Cheers Alessio

lkeegan commented 3 years ago

@alejoe91 this is the current workaround: https://github.com/ssciwr/mease-lab-to-nwb/commit/a176822dcf034273c0426a009f7b493297e01c9b

Also here are the identified rising/falling edges for the file where this issue occurs: both the first and last edge are identified as falling edges, which causes the error message:

first few points: rising-falling-start

last few points: rising-falling-end

alejoe91 commented 3 years ago

Thanks @lkeegan. The fix is exactly what I would have done. Note that the function is designed for TTL signals, while the plots you shared don't look like TTL signals at all. Is that supposed to look like that?

ross-folkard commented 3 years ago

Hi @rf13734

So that means that the acquisition was stopped before the TTL falling phase. I agree this is probably not an issue. I can push a fix excluding TTLs that are not complete. Would that be ok for you?

Cheers Alessio

Thanks @Alessio,

Have you an idea of how many TTL's may not be complete? and does this mean we risk ommiting TTL pulses that denote the onsets/offsets of trials?

ross-folkard commented 3 years ago

Thanks @lkeegan. The fix is exactly what I would have done. Note that the function is designed for TTL signals, while the plots you shared don't look like TTL signals at all. Is that supposed to look like that?

@Alessio as far as I'm aware it comes from the first 400 samples of the laser TTL trace (smrx_channel_ids = 73), have I got that right @lkeegan?

image

alejoe91 commented 3 years ago

Thanks @lkeegan. The fix is exactly what I would have done. Note that the function is designed for TTL signals, while the plots you shared don't look like TTL signals at all. Is that supposed to look like that?

@alessio as far as I'm aware it comes from the first 400 samples of the laser TTL trace (smrx_channel_ids = 73), have I got that right @lkeegan?

image

Ok then what I would do is binarize the signal. You can set all the values < max(trace) / 2 to 0 and values > max(traces) / 2 to 1. Then the diff should be much more robust. Can you try that?

lkeegan commented 2 years ago

@alejoe91 thanks for your help with this - it turned out this channel was just noise (there was no TTL data in this particular recording & channel). #48 adds a check that the channel contains valid-looking TTL data before processing it.