Merge pull request #943 from int-brain-lab/develop

release 3.3.0

.github/workflows/ibllib_ci.yml

@@ -24,6 +24,8 @@ jobs:
             python-version: 3.10
           - os: ubuntu-latest
             python-version: 3.12
+    env:
+      ONE_SAVE_ON_DELETE: false
     steps:
     - uses: actions/checkout@v2
     - name: Set up Python ${{ matrix.python-version }}

brainbox/behavior/training.py

@@ -378,7 +378,7 @@ def get_training_status(trials, task_protocol, ephys_sess_dates, n_delay):
                                         ephys_sess_dates])
             n_ephys_trials = np.array([compute_n_trials(trials[k]) for k in ephys_sess_dates])
 
-            pass_criteria, criteria = criterion_delay(n_ephys, n_ephys_trials, perf_ephys_easy)
+            pass_criteria, criteria = criterion_delay(n_ephys_trials, perf_ephys_easy, n_ephys=n_ephys)
 
             if pass_criteria:
                 status = 'ready4delay'
@@ -430,24 +430,24 @@ def display_status(subj, sess_dates, status, perf_easy=None, n_trials=None, psyc
               f"{sess_dates[2]}]")
     elif psych_20 is None:
         print(f"\n{subj} : {status} \nSession dates={[x for x in sess_dates]}, "
-              f"Perf easy={[np.around(pe,2) for pe in perf_easy]}, "
+              f"Perf easy={[np.around(pe, 2) for pe in perf_easy]}, "
               f"N trials={[nt for nt in n_trials]} "
               f"\nPsych fit over last 3 sessions: "
-              f"bias={np.around(psych[0],2)}, thres={np.around(psych[1],2)}, "
-              f"lapse_low={np.around(psych[2],2)}, lapse_high={np.around(psych[3],2)} "
+              f"bias={np.around(psych[0], 2)}, thres={np.around(psych[1], 2)}, "
+              f"lapse_low={np.around(psych[2], 2)}, lapse_high={np.around(psych[3], 2)} "
               f"\nMedian reaction time at 0 contrast over last 3 sessions = "
-              f"{np.around(rt,2)}")
+              f"{np.around(rt, 2)}")
 
     else:
         print(f"\n{subj} : {status} \nSession dates={[x for x in sess_dates]}, "
-              f"Perf easy={[np.around(pe,2) for pe in perf_easy]}, "
+              f"Perf easy={[np.around(pe, 2) for pe in perf_easy]}, "
               f"N trials={[nt for nt in n_trials]} "
               f"\nPsych fit over last 3 sessions (20): "
-              f"bias={np.around(psych_20[0],2)}, thres={np.around(psych_20[1],2)}, "
-              f"lapse_low={np.around(psych_20[2],2)}, lapse_high={np.around(psych_20[3],2)} "
-              f"\nPsych fit over last 3 sessions (80): bias={np.around(psych_80[0],2)}, "
-              f"thres={np.around(psych_80[1],2)}, lapse_low={np.around(psych_80[2],2)}, "
-              f"lapse_high={np.around(psych_80[3],2)} "
+              f"bias={np.around(psych_20[0], 2)}, thres={np.around(psych_20[1], 2)}, "
+              f"lapse_low={np.around(psych_20[2], 2)}, lapse_high={np.around(psych_20[3], 2)} "
+              f"\nPsych fit over last 3 sessions (80): bias={np.around(psych_80[0], 2)}, "
+              f"thres={np.around(psych_80[1], 2)}, lapse_low={np.around(psych_80[2], 2)}, "
+              f"lapse_high={np.around(psych_80[3], 2)} "
               f"\nMedian reaction time at 0 contrast over last 3 sessions = "
               f"{np.around(rt, 2)}")
 
@@ -997,7 +997,7 @@ def criterion_ephys(psych_20, psych_80, n_trials, perf_easy, rt):
     return passing, criteria
 
 
-def criterion_delay(n_ephys, n_trials, perf_easy):
+def criterion_delay(n_trials, perf_easy, n_ephys=1):
     """
     Returns bool indicating whether criteria for 'ready4delay' is met.
 

brainbox/ephys_plots.py

@@ -439,20 +439,22 @@ def plot_brain_regions(channel_ids, channel_depths=None, brain_regions=None, dis
             bar_kwargs.update(**kwargs)
             color = col / 255
             ax.bar(x=0.5, height=height, color=color, bottom=reg[0], **kwargs)
-        if label == 'right':
-            ax.yaxis.tick_right()
-        ax.set_yticks(region_labels[:, 0].astype(int))
-        ax.yaxis.set_tick_params(labelsize=8)
-        ax.set_ylim(np.nanmin(channel_depths), np.nanmax(channel_depths))
-        ax.get_xaxis().set_visible(False)
-        ax.set_yticklabels(region_labels[:, 1])
-        if label == 'right':
-            ax.yaxis.tick_right()
-            ax.spines['left'].set_visible(False)
-        else:
-            ax.spines['right'].set_visible(False)
         ax.spines['top'].set_visible(False)
         ax.spines['bottom'].set_visible(False)
+        if label is not None:
+            if label == 'right':
+                ax.yaxis.tick_right()
+            ax.set_yticks(region_labels[:, 0].astype(int))
+            ax.yaxis.set_tick_params(labelsize=8)
+            ax.set_ylim(np.nanmin(channel_depths), np.nanmax(channel_depths))
+            ax.get_xaxis().set_visible(False)
+            ax.set_yticklabels(region_labels[:, 1])
+            if label == 'right':
+                ax.yaxis.tick_right()
+                ax.spines['left'].set_visible(False)
+            else:
+                ax.spines['right'].set_visible(False)
+
         if title:
             ax.set_title(title)
 

brainbox/io/one.py

@@ -899,12 +899,13 @@ def load_spike_sorting_object(self, obj, *args, **kwargs):
         self.download_spike_sorting_object(obj, *args, **kwargs)
         return self._load_object(self.files[obj])
 
-    def get_version(self, spike_sorter='pykilosort'):
+    def get_version(self, spike_sorter=None):
+        spike_sorter = (spike_sorter or self.spike_sorter) or 'iblsorter'
         collection = self._get_spike_sorting_collection(spike_sorter=spike_sorter)
         dset = self.one.alyx.rest('datasets', 'list', session=self.eid, collection=collection, name='spikes.times.npy')
         return dset[0]['version'] if len(dset) else 'unknown'
 
-    def download_spike_sorting_object(self, obj, spike_sorter='pykilosort', dataset_types=None, collection=None,
+    def download_spike_sorting_object(self, obj, spike_sorter=None, dataset_types=None, collection=None,
                                       attribute=None, missing='raise', **kwargs):
         """
         Downloads an ALF object
@@ -917,6 +918,8 @@ def download_spike_sorting_object(self, obj, spike_sorter='pykilosort', dataset_
         :param missing: 'raise' (default) or 'ignore'
         :return:
         """
+        if spike_sorter is None:
+            spike_sorter = self.spike_sorter if self.spike_sorter is not None else 'iblsorter'
         if len(self.collections) == 0:
             return {}, {}, {}
         self.collection = self._get_spike_sorting_collection(spike_sorter=spike_sorter)

brainbox/tests/test_behavior.py

@@ -1,6 +1,7 @@
 from pathlib import Path
 import unittest
 from unittest import mock
+from functools import partial
 import numpy as np
 import pickle
 import copy
@@ -250,7 +251,10 @@ def test_query_criterion(self):
             'ready4ephysrig': ['2019-04-10', 'abf5109c-d780-44c8-9561-83e857c7bc01'],
             'ready4recording': ['2019-04-11', '7dc3c44b-225f-4083-be3d-07b8562885f4']
         }
-        with mock.patch.object(one.alyx, 'rest', return_value={'json': {'trained_criteria': status_map}}):
+
+        # Mock output of subjects read endpoint only
+        side_effect = partial(self._rest_mock, one.alyx.rest, {'json': {'trained_criteria': status_map}})
+        with mock.patch.object(one.alyx, 'rest', side_effect=side_effect):
             eid, n_sessions, n_days = train.query_criterion(subject, 'in_training', one=one)
             self.assertEqual('01390fcc-4f86-4707-8a3b-4d9309feb0a1', eid)
             self.assertEqual(1, n_sessions)
@@ -267,3 +271,24 @@ def test_query_criterion(self):
             self.assertIsNone(n_sessions)
             self.assertIsNone(n_days)
             self.assertRaises(ValueError, train.query_criterion, subject, 'foobar', one=one)
+
+    def _rest_mock(self, alyx_rest, return_value, *args, **kwargs):
+        """Mock return value of AlyxClient.rest function depending on input.
+
+        If using the subjects endpoint, return `return_value`. Otherwise, calls the original method.
+
+        Parameters
+        ----------
+        alyx_rest : function
+            one.webclient.AlyxClient.rest method.
+        return_value : any
+            The mock data to return.
+
+        Returns
+        -------
+        dict, list
+            Either `return_value` or the original method output.
+        """
+        if args[0] == 'subjects':
+            return return_value
+        return alyx_rest(*args, **kwargs)

examples/exploring_data/data_download.ipynb

@@ -252,8 +252,8 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "# Find an example session with data\n",
-    "eid, *_ = one.search(project='brainwide', dataset='alf/')\n",
+    "# Find an example session with trials data\n",
+    "eid, *_ = one.search(project='brainwide', dataset='_ibl_trials.table.pqt')\n",
     "# List datasets associated with a session, in the alf collection\n",
     "datasets = one.list_datasets(eid, collection='alf*')\n",
     "\n",

ibllib/__init__.py

@@ -2,7 +2,7 @@
 import logging
 import warnings
 
-__version__ = '3.2.0'
+__version__ = '3.3.0'
 warnings.filterwarnings('always', category=DeprecationWarning, module='ibllib')
 
 # if this becomes a full-blown library we should let the logging configuration to the discretion of the dev

ibllib/io/extractors/ephys_fpga.py

@@ -806,8 +806,13 @@ def _extract(self, sync=None, chmap=None, sync_collection='raw_ephys_data',
                 fcn, *_ = ibldsp.utils.sync_timestamps(bpod_start, t_trial_start)
                 buffer = 2.5  # the number of seconds to include before/after task
                 start, end = fcn(self.bpod_trials['intervals'].flat[[0, -1]])
-                tmin = min(sync['times'][0], start - buffer)
-                tmax = max(sync['times'][-1], end + buffer)
+                # NB: The following was added by k1o0 in commit b31d14e5113180b50621c985b2f230ba84da1dd3
+                # however it is not clear why this was necessary and it appears to defeat the purpose of
+                # removing the passive protocol part from the final trial extraction in ephysChoiceWorld.
+                #   tmin = min(sync['times'][0], start - buffer)
+                #   tmax = max(sync['times'][-1], end + buffer)
+                tmin = start - buffer
+                tmax = end + buffer
             else:  # This type of alignment fails for some sessions, e.g. mesoscope
                 tmin = tmax = None
 
@@ -934,22 +939,50 @@ def build_trials(self, sync, chmap, display=False, **kwargs):
         # Sync the Bpod clock to the DAQ.
         # NB: The Bpod extractor typically drops the final, incomplete, trial. Hence there is
         # usually at least one extra FPGA event. This shouldn't affect the sync. The final trial is
-        # dropped after assigning the FPGA events, using the `ifpga` index. Doing this after
+        # dropped after assigning the FPGA events, using the `ibpod` index. Doing this after
         # assigning the FPGA trial events ensures the last trial has the correct timestamps.
         self.bpod2fpga, drift_ppm, ibpod, ifpga = self.sync_bpod_clock(self.bpod_trials, fpga_events, self.sync_field)
 
-        if np.any(np.diff(ibpod) != 1) and self.sync_field == 'intervals_0':
+        bpod_start = self.bpod2fpga(self.bpod_trials['intervals'][:, 0])
+        missing_bpod_idx = np.setxor1d(ibpod, np.arange(len(bpod_start)))
+        if missing_bpod_idx.size > 0 and self.sync_field == 'intervals_0':
             # One issue is that sometimes pulses may not have been detected, in this case
             # add the events that have not been detected and re-extract the behaviour sync.
             # This is only really relevant for the Bpod interval events as the other TTLs are
             # from devices where a missing TTL likely means the Bpod event was truly absent.
             _logger.warning('Missing Bpod TTLs; reassigning events using aligned Bpod start times')
-            bpod_start = self.bpod_trials['intervals'][:, 0]
-            missing_bpod = self.bpod2fpga(bpod_start[np.setxor1d(ibpod, np.arange(len(bpod_start)))])
-            t_trial_start = np.sort(np.r_[fpga_events['intervals_0'][:, 0], missing_bpod])
+            missing_bpod = bpod_start[missing_bpod_idx]
+            # Another complication: if the first trial start is missing on the FPGA, the second
+            # trial start is assumed to be the first and is mis-assigned to another trial event
+            # (i.e. valve open). This is done because the first Bpod pulse is irregularly long.
+            # See `FpgaTrials.get_bpod_event_times` for details.
+
+            # If first trial start is missing first detected FPGA event doesn't match any Bpod
+            # starts then it's probably a mis-assigned valve or trial end event.
+            i1 = np.any(missing_bpod_idx == 0) and not np.any(np.isclose(fpga_events['intervals_0'][0], bpod_start))
+            # skip mis-assigned first FPGA trial start
+            t_trial_start = np.sort(np.r_[fpga_events['intervals_0'][int(i1):], missing_bpod])
+            ibpod = np.sort(np.r_[ibpod, missing_bpod_idx])
+            if i1:
+                # The first trial start is actually the first valve open here
+                first_on, first_off = bpod_event_intervals['trial_start'][0, :]
+                bpod_valve_open = self.bpod2fpga(self.bpod_trials['feedback_times'][self.bpod_trials['feedbackType'] == 1])
+                if np.any(np.isclose(first_on, bpod_valve_open)):
+                    # Probably assigned to the valve open
+                    _logger.debug('Re-reassigning first valve open event. TTL length = %.3g ms', first_off - first_on)
+                    fpga_events['valveOpen_times'] = np.sort(np.r_[first_on, fpga_events['valveOpen_times']])
+                    fpga_events['valveClose_times'] = np.sort(np.r_[first_off, fpga_events['valveClose_times']])
+                elif np.any(np.isclose(first_on, self.bpod2fpga(self.bpod_trials['itiIn_times']))):
+                    # Probably assigned to the trial end
+                    _logger.debug('Re-reassigning first trial end event. TTL length = %.3g ms', first_off - first_on)
+                    fpga_events['itiIn_times'] = np.sort(np.r_[first_on, fpga_events['itiIn_times']])
+                    fpga_events['intervals_1'] = np.sort(np.r_[first_off, fpga_events['intervals_1']])
+                else:
+                    _logger.warning('Unable to reassign first trial start event. TTL length = %.3g ms', first_off - first_on)
+                # Bpod trial_start event intervals are not used but for consistency we'll update them here anyway
+                bpod_event_intervals['trial_start'] = bpod_event_intervals['trial_start'][1:, :]
         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.
@@ -1000,10 +1033,12 @@ def build_trials(self, sync, chmap, display=False, **kwargs):
         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] for k in fpga_trials.keys()})
+        # Use ibpod to discard the final trial if it is incomplete
+        # ibpod should be indices of all Bpod trials, even those that were not detected on the FPGA
+        out.update({k: fpga_trials[k][ibpod] for k in fpga_trials.keys()})
 
         if display:  # pragma: no cover
-            width = 0.5
+            width = 2
             ymax = 5
             if isinstance(display, bool):
                 plt.figure('Bpod FPGA Sync')

ibllib/io/extractors/ephys_passive.py

@@ -68,9 +68,11 @@ def _load_passive_session_fixtures(session_path: str, task_collection: str = 'ra
     :rtype: dict
     """
 
-    # THIS CAN BE PREGENERATED SESSION NO
+    # The pregenerated session number has had many parameter names
     settings = rawio.load_settings(session_path, task_collection=task_collection)
-    ses_nb = settings['PREGENERATED_SESSION_NUM']
+    pars = map(settings.get, ['PRELOADED_SESSION_NUM', 'PREGENERATED_SESSION_NUM', 'SESSION_TEMPLATE_ID'])
+    ses_nb = next((k for k in pars if k is not None), None)
+
     session_order = settings.get('SESSION_ORDER', None)
     if session_order:  # TODO test this out and make sure it okay
         assert settings["SESSION_ORDER"][settings["SESSION_IDX"]] == ses_nb

ibllib/io/video.py

@@ -151,12 +151,26 @@ def get_video_meta(video_path, one=None):
 
 
 def url_from_eid(eid, label=None, one=None):
-    """Return the video URL(s) for a given eid
-
-    :param eid: The session id
-    :param label: The video label (e.g. 'body') or a tuple thereof
-    :param one: An instance of ONE
-    :return: The URL string if the label is a string, otherwise a dict of urls with labels as keys
+    """Return the video URL(s) for a given eid.
+
+    Parameters
+    ----------
+    eid : UUID, str
+        The session ID.
+    label : str, tuple of str
+        The video label (e.g. 'body') or a tuple thereof.
+    one : one.api.One
+        An instance of ONE.
+
+    Returns
+    -------
+    str, dict of str
+        The URL string if the label is a string, otherwise a dict of urls with labels as keys.
+
+    Raises
+    ------
+    ValueError
+        Video label is unreckognized.  See `VIDEO_LABELS` for valid labels.
     """
     valid_labels = VIDEO_LABELS
     if not (label is None or np.isin(label, valid_labels).all()):

ibllib/oneibl/data_handlers.py

@@ -201,7 +201,7 @@ def filter(self, session_datasets, **kwargs):
         Parameters
         ----------
         session_datasets : pandas.DataFrame
-            An data frame of session datasets.
+            A data frame of session datasets.
         kwargs
             Extra arguments for `one.util.filter_datasets`, namely revision_last_before, qc, and
             ignore_qc_not_set.
@@ -766,7 +766,7 @@ def setUp(self, **_):
         :return:
         """
         df = super().getData()
-        self.one._check_filesystem(df)
+        self.one._check_filesystem(df, check_hash=False)
 
     def uploadData(self, outputs, version, **kwargs):
         """