search

carrien / free-speech

Analysis and plotting code for speech neuroimaging experiments.
MIT License
4 stars 5 forks source link

check_audio saves signalIn and signalOut info #12

Closed cwnaber closed 3 years ago

cwnaber commented 3 years ago

Made a new, 6th input parameter for check_audio.m that accepts either 'signalIn' (default) or 'signalOut'. Pretty straightforward. I did some light testing and it worked.

I took the variable name 'buffertype' from waverunner.m, where that variable serves the same purpose as here.

lhantzsch commented 3 years ago

As of now, any time a trial is marked bad in 'signalIn', the corresponding trial of 'signalOut' is automatically also marked bad. This is also true in the other direction (any 'signalOut' trial marked as bad causes the corresponding 'signalIn' trial to be marked bad). Is this OK? (as there will usually be trials in which the 'signalIn' data is good, but the 'signalOut' has clipped)

cwnaber commented 3 years ago

As of now, any time a trial is marked bad in 'signalIn', the corresponding trial of 'signalOut' is automatically also marked bad. This is also true in the other direction (any 'signalOut' trial marked as bad causes the corresponding 'signalIn' trial to be marked bad). Is this OK? (as there will usually be trials in which the 'signalIn' data is good, but the 'signalOut' has clipped)

I'm pretty sure that's what we want -- either both signals are good, or we need to mark the whole trial as bad. @carrien or @parrell can you just confirm?

parrell commented 3 years ago

I think we want marking bad trials in signalIn to automatically mark them as bad in signalOut, but not the other way around. Bad trials in signalIn mean something is bad about the production (cut off, wrong word, cough, etc.) whereas bad trials in signalOut that are fine in signal in are about audapter cutting out, but the produced speech is still ok.

Edit: thinking about this a little more deeply, I guess bad trials in signalOut mean the participant didn't get the expected feedback. I think whether we want to include that trial will depend on the particular experiment. So, I still like what I am proposing as the default so it's easy to include these trials, but for particular experiments some folks may want to do a little more complicated indexing.

cwnaber commented 3 years ago

I think we'd have to make changes to the data structure to differentiate one of the signal directions being good while the other is bad. As I can tell, we store good/bad status in the trials folder -> individual trial file 12.mat -> trialparams.event_params.is_good_trial, where is_good_trial is a binary integer; no distinction beyond the trial itself.

carrien commented 3 years ago

Two thoughts here: First, we don't necessarily need to make changes to the data structure to keep track of bad signalOut trials separately. For example, if you look at how audioGUI handles the buffertype argument, it uses a different "trials" folder for buffertypes that aren't signalIn (e.g., it will put tracks from signalOut in trials_signalOut). So if a user uses audioGUI to mark signalOut trial 12 as bad, trials_signalOut/12.mat has is_good_trial is set to 0, but it doesn't touch trials/12.mat. We can keep the parallelism there if we want.

The second thought, though, is that we may want trials that are bad in signalOut to also be bad in signalIn. In the arrangement above where things are kept separate, we can add a script that transfers the "bad" labels back and forth -- but if the normal use case is that they should be in sync, then we want to think about changes to the signalOut workflow that would make this more automatic.

parrell commented 3 years ago

It's too bad we have is_good_trial. If we had is_bad_trial we could set it to 1 for signalIn and 2 for signalOut. Seems like this should be a discussion at lab meeting Tuesday.

cwnaber commented 3 years ago

(This is mainly to @carrien and @parrell ) Per our lab discussion yesterday, we want to move forward with some aspect of this functionality. We want users to be able to use check_audio on signalOut, which should be saved to the trials_signalOut folder. For signalIn, using the trials folder, is_good_trial can be 1 while the same flag is 0 in the trials_signalOut folder can be 0. We do not want any is_good_trial determination to apply onto another signal type.

We're unsure what to do long term about dataVals.bExcl. However, current behavior for check_audio is that any time a trial's good or bad status is changed, it gets a statusChange flag. When the user saves, the value of that trial (good or bad) gets stored in dataVals.bExcl and overwrites any previous value. It seems like for now, we would want the behavior to be this:

Is that an accurate summary, and if so is that second bullet point the appropriate behavior for updating dataVals.bExcl for the time being?

cwnaber commented 3 years ago

Intended behavior for this PR changed since inception. Now, the goal is to keep is_good_trial for signalIn and signalOut in sync. Since there are separate dataVals files that get created for signalIn and signalOut by gen_dataVals_for_wave_viewer (there's a file dataVals and a file dataVals_signalOut), bExcl can technically be unique for signalIn and signalOut. However, check_audio will always have signalIn and signalOut in sync.

With these changes, check_audio now displays the waveform for signalIn and signalOut simultaneously. The Play button in check_audio only ever plays the signalIn audio. When saving, check_audio creates a "trials" and a "trials_signalOut" folder, whose contents will be identical. If a signalOut is missing, the figure for it will be blank. See trials 1-3 in attached image. image

Some minor QOL changes were introduced as well: taller y-axis limits on waveforms, x-axis limits on waveform fit to the edge of the figure, and code comments were added.