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 <[email protected]>
Co-authored-by: Miles Wells <[email protected]>

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

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

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,16 +193,15 @@ 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)
         data['errorCueTrigger_times'] = outcome_times(data['feedback_times'], ~data['correct'])
         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

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):