From 0a7a59e44003804501777a1a13627db1c608974a Mon Sep 17 00:00:00 2001 From: Olivier Winter Date: Mon, 7 Oct 2024 12:16:57 +0100 Subject: [PATCH] Qc fixes (#852) * get_protocol_period: relax assertion for spacer_times * fix stim freeze indexing issue in ephys_fpga extraction * change ITI constants from 1s to 500ms * proposes fix for dynamic pipeline test * remove test already covered by test_io * flake8 * ITI qc for no-go trials * check_negative_feedback_stimOff_delays also needed some fixing --------- Co-authored-by: Florian Rau Co-authored-by: Miles Wells --- ibllib/io/extractors/ephys_fpga.py | 11 ++++++----- ibllib/qc/task_metrics.py | 21 ++++++++++++++------- ibllib/tests/qc/test_task_metrics.py | 9 ++++----- ibllib/tests/test_dynamic_pipeline.py | 10 ---------- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/ibllib/io/extractors/ephys_fpga.py b/ibllib/io/extractors/ephys_fpga.py index 3810abf29..e7ae93220 100644 --- a/ibllib/io/extractors/ephys_fpga.py +++ b/ibllib/io/extractors/ephys_fpga.py @@ -569,7 +569,8 @@ def get_protocol_period(session_path, protocol_number, bpod_sync): # Ensure that the number of detected spacers matched the number of expected tasks if acquisition_description := session_params.read_params(session_path): n_tasks = len(acquisition_description.get('tasks', [])) - assert n_tasks == len(spacer_times), f'expected {n_tasks} spacers, found {len(spacer_times)}' + assert len(spacer_times) >= protocol_number, (f'expected {n_tasks} spacers, found only {len(spacer_times)} - ' + f'can not return protocol number {protocol_number}.') assert n_tasks > protocol_number >= 0, f'protocol number must be between 0 and {n_tasks}' else: assert protocol_number < len(spacer_times) @@ -935,6 +936,7 @@ def build_trials(self, sync, chmap, display=False, **kwargs): t_trial_start = np.sort(np.r_[fpga_events['intervals_0'][:, 0], missing_bpod]) else: t_trial_start = fpga_events['intervals_0'] + t_trial_start = t_trial_start[ifpga] out = alfio.AlfBunch() # Add the Bpod trial events, converting the timestamp fields to FPGA time. @@ -959,9 +961,9 @@ def build_trials(self, sync, chmap, display=False, **kwargs): # f2ttl times are unreliable owing to calibration and Bonsai sync square update issues. # Take the first event after the FPGA aligned stimulus trigger time. - fpga_trials['stimOn_times'][ibpod] = _assign_events_to_trial( + fpga_trials['stimOn_times'] = _assign_events_to_trial( out['stimOnTrigger_times'], f2ttl_t, take='first', t_trial_end=out['stimOffTrigger_times']) - fpga_trials['stimOff_times'][ibpod] = _assign_events_to_trial( + fpga_trials['stimOff_times'] = _assign_events_to_trial( out['stimOffTrigger_times'], f2ttl_t, take='first', t_trial_end=out['intervals'][:, 1]) # For stim freeze we take the last event before the stim off trigger time. # To avoid assigning early events (e.g. for sessions where there are few flips due to @@ -980,13 +982,12 @@ def build_trials(self, sync, chmap, display=False, **kwargs): # take last event after freeze/stim on trigger, before stim off trigger stim_freeze = _assign_events_to_trial(lims, f2ttl_t, take='last', t_trial_end=out['stimOffTrigger_times']) fpga_trials['stimFreeze_times'][go_trials] = stim_freeze[go_trials] - # Feedback times are valve open on correct trials and error tone in on incorrect trials fpga_trials['feedback_times'] = np.copy(fpga_trials['valveOpen_times']) ind_err = np.isnan(fpga_trials['valveOpen_times']) fpga_trials['feedback_times'][ind_err] = fpga_trials['errorCue_times'][ind_err] - out.update({k: fpga_trials[k][ifpga] for k in fpga_trials.keys()}) + out.update({k: fpga_trials[k] for k in fpga_trials.keys()}) if display: # pragma: no cover width = 0.5 diff --git a/ibllib/qc/task_metrics.py b/ibllib/qc/task_metrics.py index c510d271c..f5ff83437 100644 --- a/ibllib/qc/task_metrics.py +++ b/ibllib/qc/task_metrics.py @@ -65,6 +65,9 @@ _log = logging.getLogger(__name__) +# todo the 2 followint parameters should be read from the task parameters for each session +ITI_DELAY_SECS = .5 +FEEDBACK_NOGO_DELAY_SECS = 2 BWM_CRITERIA = { 'default': {'PASS': 0.99, 'WARNING': 0.90, 'FAIL': 0}, # Note: WARNING was 0.95 prior to Aug 2022 @@ -647,7 +650,8 @@ def check_stimOff_itiIn_delays(data, **_): return metric, passed -def check_iti_delays(data, subtract_pauses=False, **_): +def check_iti_delays(data, subtract_pauses=False, iti_delay_secs=ITI_DELAY_SECS, + feedback_nogo_delay_secs=FEEDBACK_NOGO_DELAY_SECS, **_): """ Check the open-loop grey screen period is approximately 1 second. @@ -678,16 +682,17 @@ def check_iti_delays(data, subtract_pauses=False, **_): numpy.array An array of boolean values, 1 per trial, where True means trial passes QC threshold. """ - # Initialize array the length of completed trials - ITI = 1. metric = np.full(data['intervals'].shape[0], np.nan) passed = metric.copy() pauses = (data['pause_duration'] if subtract_pauses else np.zeros_like(metric))[:-1] # Get the difference between stim off and the start of the next trial # Missing data are set to Inf, except for the last trial which is a NaN - metric[:-1] = \ - np.nan_to_num(data['intervals'][1:, 0] - data['stimOff_times'][:-1] - ITI - pauses, nan=np.inf) - passed[:-1] = np.abs(metric[:-1]) < (ITI / 10) # Last trial is not counted + metric[:-1] = np.nan_to_num( + data['intervals'][1:, 0] - data['stimOff_times'][:-1] - iti_delay_secs - pauses, + nan=np.inf + ) + metric[data['choice'] == 0] = metric[data['choice'] == 0] - feedback_nogo_delay_secs + passed[:-1] = np.abs(metric[:-1]) < (iti_delay_secs / 10) # Last trial is not counted assert data['intervals'].shape[0] == len(metric) == len(passed) return metric, passed @@ -720,7 +725,7 @@ def check_positive_feedback_stimOff_delays(data, **_): return metric, passed -def check_negative_feedback_stimOff_delays(data, **_): +def check_negative_feedback_stimOff_delays(data, feedback_nogo_delay_secs=FEEDBACK_NOGO_DELAY_SECS, **_): """ Check the stimulus offset occurs approximately 2 seconds after negative feedback delivery. @@ -739,6 +744,8 @@ def check_negative_feedback_stimOff_delays(data, **_): :param data: dict of trial data with keys ('stimOff_times', 'errorCue_times', 'intervals') """ metric = np.nan_to_num(data['stimOff_times'] - data['errorCue_times'] - 2, nan=np.inf) + # for the nogo trials, the feedback is the same as the stimOff + metric[data['choice'] == 0] = metric[data['choice'] == 0] + feedback_nogo_delay_secs # Apply criteria passed = (np.abs(metric) < 0.15).astype(float) # Remove none negative feedback trials diff --git a/ibllib/tests/qc/test_task_metrics.py b/ibllib/tests/qc/test_task_metrics.py index a0433332a..d658302c4 100644 --- a/ibllib/tests/qc/test_task_metrics.py +++ b/ibllib/tests/qc/test_task_metrics.py @@ -159,10 +159,10 @@ def load_fake_bpod_data(n=5): # add a 5s pause on 3rd trial pauses[2] = 5. quiescence_length = 0.2 + np.random.standard_exponential(size=(n,)) - iti_length = 1 # inter-trial interval + iti_length = .5 # inter-trial interval # trial lengths include quiescence period, a couple small trigger delays and iti trial_lengths = quiescence_length + resp_feeback_delay + (trigg_delay * 4) + iti_length - # add on 60s for nogos + feedback time (1 or 2s) + ~0.5s for other responses + # add on 60 + 2s for nogos + feedback time (1 or 2s) + ~0.5s for other responses trial_lengths += (choice == 0) * 60 + (~correct + 1) + (choice != 0) * N(0.5) start_times = (np.r_[0, np.cumsum(trial_lengths)] + np.r_[0, np.cumsum(pauses)])[:-1] end_times = np.cumsum(trial_lengths) - 1e-2 + np.r_[0, np.cumsum(pauses)][:-1] @@ -193,8 +193,8 @@ def load_fake_bpod_data(n=5): outcome = data['feedbackType'].copy() outcome[data['choice'] == 0] = 0 data['outcome'] = outcome - # Delay of 1 second if correct, 2 seconds if incorrect - data['stimOffTrigger_times'] = data['feedback_times'] + (~correct + 1) + # Delay of 1 second if correct, 2 seconds if incorrect, and stim off at feedback for nogo + data['stimOffTrigger_times'] = data['feedback_times'] + (~correct + 1) - (choice == 0) * 2 data['stimOff_times'] = data['stimOffTrigger_times'] + trigg_delay # Error tone times nan on incorrect trials outcome_times = np.vectorize(lambda x, y: x + 1e-2 if y else np.nan) @@ -202,7 +202,6 @@ def load_fake_bpod_data(n=5): data['errorCue_times'] = data['errorCueTrigger_times'] + trigg_delay data['valveOpen_times'] = outcome_times(data['feedback_times'], data['correct']) data['rewardVolume'] = ~np.isnan(data['valveOpen_times']) * 3.0 - return data @staticmethod diff --git a/ibllib/tests/test_dynamic_pipeline.py b/ibllib/tests/test_dynamic_pipeline.py index 1a4436057..3a097236c 100644 --- a/ibllib/tests/test_dynamic_pipeline.py +++ b/ibllib/tests/test_dynamic_pipeline.py @@ -9,19 +9,9 @@ import ibllib.tests import ibllib.pipes.dynamic_pipeline as dyn import ibllib.pipes.behavior_tasks as btasks -from ibllib.io import session_params from ibllib.tests.fixtures.utils import populate_task_settings -def test_read_write_params_yaml(): - ad = dyn.get_acquisition_description('choice_world_recording') - with tempfile.TemporaryDirectory() as td: - session_path = Path(td) - session_params.write_params(session_path, ad) - add = session_params.read_params(session_path) - assert ad == add - - class TestCreateLegacyAcqusitionDescriptions(unittest.TestCase): def test_legacy_biased(self):