LorenFrankLab / spyglass

Neuroscience data analysis framework for reproducible research built by Loren Frank Lab at UCSF
https://lorenfranklab.github.io/spyglass/
MIT License
89 stars 41 forks source link

`_compute_isi_violation_fractions` returns a dict for each unit #1080

Closed emreybroyles closed 21 hours ago

emreybroyles commented 2 weeks ago

Describe the bug Code in spikesorting quality metrics tries to create 1 floating point number out of a dictionary.

When running the line sgss.QualityMetrics.populate([(sgss.MetricSelection & metric_key).proj()]) I get the error

File ~/Src/spyglass/src/spyglass/spikesorting/v0/spikesorting_curation.py:605, in QualityMetrics._dump_to_json(self, qm_dict, save_path)
    603     m = {}
    604     for unit_id, metric_val in value.items():
--> 605         m[str(unit_id)] = np.float64(metric_val)
    606     new_qm[str(key)] = m
    607 with open(save_path, "w", encoding="utf-8") as f:

TypeError: float() argument must be a string or a number, not 'dict'

where

metric_val = {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154}

and

value.items() = dict_items([('1', {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154}), ('2', {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154}), ('3', {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154})])

To Reproduce Steps to reproduce the behavior:

  1. This error is on file 01_spikesorting_batch_SC-Error.ipynb at file path /home/ebroyles/Src/spyglass/notebooks/eb_notebooks/01_spikesorting_batch_SC-Error.ipynb
  2. See error in cell 28
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In [28], line 95
     93 metric_key.update({"metric_params_name": metric_params_name})
     94 sgss.MetricSelection.insert1(metric_key, skip_duplicates=True)
---> 95 sgss.QualityMetrics.populate([(sgss.MetricSelection & metric_key).proj()])
     97 # Automatic curation
     98 autocuration_key = metric_key.copy()

File ~/Src/spyglass/src/spyglass/utils/dj_mixin.py:713, in SpyglassMixin.populate(self, *restrictions, **kwargs)
    711 if processes == 1 or not self._parallel_make:
    712     kwargs["processes"] = processes
--> 713     return super().populate(*restrictions, **kwargs)
    715 # If parallel in both make and populate, use non-daemon processes
    716 # Get keys to populate
    717 keys = (self._jobs_to_do(restrictions) - self.target).fetch(
    718     "KEY", limit=kwargs.get("limit", None)
    719 )

File ~/anaconda3/envs/spyglass/lib/python3.9/site-packages/datajoint/autopopulate.py:241, in AutoPopulate.populate(self, suppress_errors, return_exception_objects, reserve_jobs, order, limit, max_calls, display_progress, processes, make_kwargs, *restrictions)
    237 if processes == 1:
    238     for key in (
    239         tqdm(keys, desc=self.__class__.__name__) if display_progress else keys
    240     ):
--> 241         error = self._populate1(key, jobs, **populate_kwargs)
    242         if error is not None:
    243             error_list.append(error)

File ~/anaconda3/envs/spyglass/lib/python3.9/site-packages/datajoint/autopopulate.py:292, in AutoPopulate._populate1(self, key, jobs, suppress_errors, return_exception_objects, make_kwargs)
    290 self.__class__._allow_insert = True
    291 try:
--> 292     make(dict(key), **(make_kwargs or {}))
    293 except (KeyboardInterrupt, SystemExit, Exception) as error:
    294     try:

File ~/Src/spyglass/src/spyglass/spikesorting/v0/spikesorting_curation.py:540, in QualityMetrics.make(self, key)
    538 # save metrics dict as json
    539 logger.info(f"Computed all metrics: {qm}")
--> 540 self._dump_to_json(qm, key["quality_metrics_path"])
    542 key["object_id"] = AnalysisNwbfile().add_units_metrics(
    543     key["analysis_file_name"], metrics=qm
    544 )
    545 AnalysisNwbfile().add(key["nwb_file_name"], key["analysis_file_name"])

File ~/Src/spyglass/src/spyglass/spikesorting/v0/spikesorting_curation.py:605, in QualityMetrics._dump_to_json(self, qm_dict, save_path)
    603     m = {}
    604     for unit_id, metric_val in value.items():
--> 605         m[str(unit_id)] = np.float64(metric_val)
    606     new_qm[str(key)] = m
    607 with open(save_path, "w", encoding="utf-8") as f:

TypeError: float() argument must be a string or a number, not 'dict'
samuelbray32 commented 2 weeks ago

just a default check; what version of spikeinterface do you have in your environment?

emreybroyles commented 2 weeks ago

The version was 0.99.1. So I just updated it to spikeinterface-0.101.0 just in case; also did a git pull. But now i am getting an import error from spyglass.common. I tested on virga-03, breeze, and zephyr. Sorry idk how to get rid of the "vscode-notebook-cell..." lines ... Let me know if i should install a specific version!

Error stack ```python --------------------------------------------------------------------------- AttributeError Traceback (most recent call last) Cell In [1], [line 5](vscode-notebook-cell:?execution_count=1&line=5) [3](notebook-cell:&line=3) import datajoint as dj [4](notebook-cell:&line=4) import spyglass as sg ----> [5](notebook-cell:&line=5) import spyglass.common as sgc [6](notebook-cell:&line=6) import spyglass.spikesorting.v0 as sgss # 8/8/24: SC checking with Sam which version corresponds to our prior spikesorting [7](notebook-cell:&line=7) import spyglass.data_import as sdi File ~/Src/spyglass/src/spyglass/common/__init__.py:3 [1](/spyglass/common/__init__.py:1) from spyglass.utils.dj_mixin import SpyglassMixin # isort:skip ----> [3](/spyglass/common/__init__.py:3) from spyglass.common.common_behav import ( [4](/spyglass/common/__init__.py:4) PositionIntervalMap, [5](/spyglass/common/__init__.py:5) PositionSource, [6](/spyglass/common/__init__.py:6) RawPosition, [7](/spyglass/common/__init__.py:7) StateScriptFile, [8](/spyglass/common/__init__.py:8) VideoFile, [9](/spyglass/common/__init__.py:9) convert_epoch_interval_name_to_position_interval_name, [10](/spyglass/common/__init__.py:10) ) [11](/spyglass/common/__init__.py:11) from spyglass.common.common_device import ( [12](/spyglass/common/__init__.py:12) CameraDevice, [13](/spyglass/common/__init__.py:13) DataAcquisitionDevice, (...) [17](/spyglass/common/__init__.py:17) ProbeType, [18](/spyglass/common/__init__.py:18) ) [19](/spyglass/common/__init__.py:19) from spyglass.common.common_dio import DIOEvents File ~/Src/spyglass/src/spyglass/common/common_behav.py:12 [9](/spyglass/common/common_behav.py:9) import pynwb [11](/spyglass/common/common_behav.py:11) from spyglass.common.common_device import CameraDevice ---> [12](/spyglass/common/common_behav.py:12) from spyglass.common.common_ephys import Raw # noqa: F401 [13](/spyglass/common/common_behav.py:13) from spyglass.common.common_interval import IntervalList, interval_list_contains [14](/spyglass/common/common_behav.py:14) from spyglass.common.common_nwbfile import Nwbfile File ~/Src/spyglass/src/spyglass/common/common_ephys.py:11 [9](/spyglass/common/common_ephys.py:9) from spyglass.common.common_device import Probe # noqa: F401 [10](/spyglass/common/common_ephys.py:10) from spyglass.common.common_filter import FirFilterParameters ---> [11](/spyglass/common/common_ephys.py:11) from spyglass.common.common_interval import interval_list_censor # noqa: F401 [12](/spyglass/common/common_ephys.py:12) from spyglass.common.common_interval import ( [13](/spyglass/common/common_ephys.py:13) IntervalList, [14](/spyglass/common/common_ephys.py:14) interval_list_contains_ind, [15](/spyglass/common/common_ephys.py:15) interval_list_intersect, [16](/spyglass/common/common_ephys.py:16) ) [17](/spyglass/common/common_ephys.py:17) from spyglass.common.common_nwbfile import AnalysisNwbfile, Nwbfile File ~/Src/spyglass/src/spyglass/common/common_interval.py:10 [7](/spyglass/common/common_interval.py:7) import pandas as pd [8](/spyglass/common/common_interval.py:8) from pynwb import NWBFile ---> [10](/spyglass/common/common_interval.py:10) from spyglass.common.common_session import Session # noqa: F401 [11](/spyglass/common/common_interval.py:11) from spyglass.utils import SpyglassMixin, logger [12](/spyglass/common/common_interval.py:12) from spyglass.utils.dj_helper_fn import get_child_tables File ~/Src/spyglass/src/spyglass/common/common_session.py:8 [1](/spyglass/common/common_session.py:1) import datajoint as dj [3](/spyglass/common/common_session.py:3) from spyglass.common.common_device import ( [4](/spyglass/common/common_session.py:4) CameraDevice, [5](/spyglass/common/common_session.py:5) DataAcquisitionDevice, [6](/spyglass/common/common_session.py:6) Probe, [7](/spyglass/common/common_session.py:7) ) ----> [8](/spyglass/common/common_session.py:8) from spyglass.common.common_lab import Institution, Lab, LabMember [9](/spyglass/common/common_session.py:9) from spyglass.common.common_nwbfile import Nwbfile [10](/spyglass/common/common_session.py:10) from spyglass.common.common_subject import Subject File ~/Src/spyglass/src/spyglass/common/common_lab.py:8 [5](/spyglass/common/common_lab.py:5) from spyglass.utils import SpyglassMixin, logger [7](/spyglass/common/common_lab.py:7) from ..utils.nwb_helper_fn import get_nwb_file ----> [8](/spyglass/common/common_lab.py:8) from .common_nwbfile import Nwbfile [10](/spyglass/common/common_lab.py:10) schema = dj.schema("common_lab") [13](/spyglass/common/common_lab.py:13) @schema [14](/spyglass/common/common_lab.py:14) class LabMember(SpyglassMixin, dj.Manual): File ~/Src/spyglass/src/spyglass/common/common_nwbfile.py:158 [149](/spyglass/common/common_nwbfile.py:149) schema.external["raw"].delete(delete_external_files=delete_files) [152](/spyglass/common/common_nwbfile.py:152) # TODO: add_to_kachery will not work because we can't update the entry after [153](/spyglass/common/common_nwbfile.py:153) # it's been used in another table. We therefore need another way to keep track [154](/spyglass/common/common_nwbfile.py:154) # of the file here [157](/spyglass/common/common_nwbfile.py:157) @schema --> [158](/spyglass/common/common_nwbfile.py:158) class AnalysisNwbfile(SpyglassMixin, dj.Manual): [159](/spyglass/common/common_nwbfile.py:159) definition = """ [160](/spyglass/common/common_nwbfile.py:160) # Table for NWB files that contain results of analysis. [161](/spyglass/common/common_nwbfile.py:161) analysis_file_name: varchar(64) # name of the file (...) [168](/spyglass/common/common_nwbfile.py:168) INDEX (analysis_file_abs_path) [169](/spyglass/common/common_nwbfile.py:169) """ [170](/spyglass/common/common_nwbfile.py:170) # NOTE the INDEX above is implicit from filepath@... [171](/spyglass/common/common_nwbfile.py:171) # above but needs to be explicit so that alter() can work [172](/spyglass/common/common_nwbfile.py:172) [173](/spyglass/common/common_nwbfile.py:173) # See #630, #664. Excessive key length. File ~/Src/spyglass/src/spyglass/common/common_nwbfile.py:530, in AnalysisNwbfile() [524](/spyglass/common/common_nwbfile.py:524) io.write(nwbf) [525](/spyglass/common/common_nwbfile.py:525) return nwbf.units.object_id, waveforms_object_id [527](/spyglass/common/common_nwbfile.py:527) def add_units_waveforms( [528](/spyglass/common/common_nwbfile.py:528) self, [529](/spyglass/common/common_nwbfile.py:529) analysis_file_name: str, --> [530](/spyglass/common/common_nwbfile.py:530) waveform_extractor: si.WaveformExtractor, [531](/spyglass/common/common_nwbfile.py:531) metrics: dict = None, [532](/spyglass/common/common_nwbfile.py:532) labels: dict = None, [533](/spyglass/common/common_nwbfile.py:533) ): [534](/spyglass/common/common_nwbfile.py:534) """Add units to analysis NWB file along with the waveforms [535](/spyglass/common/common_nwbfile.py:535) [536](/spyglass/common/common_nwbfile.py:536) Parameters (...) [549](/spyglass/common/common_nwbfile.py:549) The NWB object id of the Units object [550](/spyglass/common/common_nwbfile.py:550) """ [552](/spyglass/common/common_nwbfile.py:552) with pynwb.NWBHDF5IO( [553](/spyglass/common/common_nwbfile.py:553) path=self.get_abs_path(analysis_file_name), [554](/spyglass/common/common_nwbfile.py:554) mode="a", [555](/spyglass/common/common_nwbfile.py:555) load_namespaces=True, [556](/spyglass/common/common_nwbfile.py:556) ) as io: AttributeError: module 'spikeinterface' has no attribute 'WaveformExtractor' ```

EDIT: formatting by CB

CBroz1 commented 2 weeks ago

I think 99.1 is the version of spikeinterface we want. That's the version that has a WaveformExtractor

The original stack suggests to me that something about qm shouldn't be there. It would be helpful to either see what qm is with %debug or have a snippet I can run to do the same myself

emreybroyles commented 1 week ago

thanks Chris, I reinstalled spikeinterface to 99.1.

here is qm: {'snr': {1: 3.4406157, 2: 3.4431686, 3: 3.453175}, 'isi_violation': {'1': {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154}, '2': {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154}, '3': {'1': 0.03923002662142001, '2': 0.04031183557760454, '3': 0.045673686881502154}}, 'nn_isolation': {'1': 0.9829000000000001, '2': 0.9776999999999999, '3': 0.9763999999999999}, 'nn_noise_overlap': {'1': 0.5006, '2': 0.4776, '3': 0.5015}, 'peak_offset': {1: 0, 2: 0, 3: 0}, 'num_spikes': {'1': 34184, '2': 35276, '3': 35470}}

the full error is here as well /home/ebroyles/Src/spyglass/notebooks/eb_notebooks/01_spikesorting_batch_SC-Error.ipynb

CBroz1 commented 4 days ago

Hi @emreybroyles - What was the value of metric_key when your first got this error?

EDIT: Found it

metric_key = {
    "curation_id": 0,
    "nwb_file_name": "SC3820230606_copy_.nwb",
    "sort_group_id": 14,
    "sort_interval_name": "pos 1 valid times",
    "preproc_params_name": "franklab_tetrode_hippocampus_min_seg",
    "team_name": "sc_eb",
    "sorter": "mountainsort4",
    "sorter_params_name": "franklab_tetrode_hippocampus_30KHz_tmp",
    "artifact_removed_interval_list_name": "SC3820230606_copy_.nwb_pos 1 valid times_100_franklab_tetrode_hippocampus_min_seg_ampl_100_prop_02_2ms_artifact_removed_valid_times",
    "waveform_params_name": "default_whitened",
    "metric_params_name": "peak_offest_num_spikes_2",
}
CBroz1 commented 4 days ago
  1. the generic compute_metric assigns a metric value here that it then overwrites here
  2. metrics are then calculated for each unit here
  3. _compute_isi_violation_fractions then returns a dict of all units, rather than just the one here
CBroz1 commented 3 days ago

Bug introduced in #1053

CBroz1 commented 21 hours ago

Resolved by #1099