Ibl exporter #3522

jonahpearl · 2024-11-06T17:59:13Z

Function for #2843
Would be great if a few others could test on their data. Thanks!

Co-authored-by: Garcia Samuel <[email protected]>

…release Prepare minor 0.100.1 release

…ug-fixes Add batch size to KS4 wrapper

for more information, see https://pre-commit.ci

…release Prepare 0.100.2

…_similarity_diff_units Fix template similarity when there are multiple sizes of units

Prepare minor release 0.100.3

Prepare 0.100.4

Preprare 0.100.5

Port bug fix PRs from Main over to 0.100-bug-fixes

Prepare release 0.100.6

for more information, see https://pre-commit.ci

zm711 · 2024-11-07T00:08:11Z

This looks like it is off of the bug fixes branch. @jonahpearl you want to switch this to main instead?

jonahpearl · 2024-11-07T02:44:40Z

I'm not 100% sure what you mean. Do you want me to cherry-pick / rebase my changes onto the main branch? I branched it a while ago so I've had to merge main in a few times.

.github/workflows/core-test.yml

alejoe91 · 2024-11-07T09:27:35Z

@jonahpearl thanks for this! Is it ok if I push to this PR?

I've been working on refining the export function so I'd be good to be in sync :)

alejoe91 · 2024-11-07T09:28:21Z

This looks like it is off of the bug fixes branch. @jonahpearl you want to switch this to main instead?

I think it's correctly using the SortingAnalyzer. Maybe the added branch in the actions yaml was confusing, so let's just remove it

alejoe91 · 2024-11-07T09:31:29Z

src/spikeinterface/exporters/to_phy.py

@@ -194,7 +194,7 @@ def export_to_phy(
        templates[unit_ind, :, :][:, : len(chan_inds)] = template
        templates_ind[unit_ind, : len(chan_inds)] = chan_inds

-    if not sorting_analyzer.has_extension("template_similarity"):
+    if sorting_analyzer.get_extension("template_similarity") is None:


You're suggesting we put this back to "has_extension"? Any reason this one and not the other ones? The ibl export relies on doing a Phy export, and as discussed elsewhere this "has_extension" pattern is no longer safe if the folder exists but without any data inside.

yes, all of them. I think it's much easier to read :)

I agree that it'd be easier to read, but I disagree that it's preferable, because it will break if the extension folder exists but doesn't have data. It's not unusual to end up in that kind of state — eg if you run analyzer.compute(["random_spikes", "spike_amplitudes"]) on a new analyzer, the spike_amplitudes folder will get made but nothing will be in it because the analyzer will need "templates" to be computed. There are various ways to fix that involving try/excepts / clean-ups, but in the current context, I don't think it makes sense to not catch this case.

That's a separate issue :) we should delay folder creation until dependencies are checked. Can you open another issue about it?

alejoe91 · 2024-11-07T09:31:49Z

src/spikeinterface/postprocessing/tests/test_template_similarity.py

+from spikeinterface.core import extract_waveforms
+from spikeinterface.extractors import toy_example
+from spikeinterface.comparison import compare_templates
+
+import numpy as np


this belongs to a different PR

Yes, not sure how this slipped in.

alejoe91 · 2024-11-07T14:55:49Z

src/spikeinterface/exporters/to_ibl.py

+
+import numpy as np
+import numpy.typing as npt
+from scipy.signal import welch


import scipy should be done within the functions using it because it's not a mandatory requirement (see failing tests)

Co-authored-by: Alessio Buccino <[email protected]>

for more information, see https://pre-commit.ci

zm711

Just a docstring fix while you're at it. I believe we add this to the dev docs after this PR was started which is why this was missed.

zm711 · 2024-11-07T18:41:22Z

src/spikeinterface/exporters/to_ibl.py

+    ----------
+    analyzer: SortingAnalyzer
+        The sorting analyzer object.
+    output_folder: str | Path
+        The output folder where the phy template-gui files are saved
+    rms_win_length_sec: float, default: 3
+        The window length in seconds for the RMS calculation.
+    welch_win_length_samples: int, default: 1024
+        The window length in samples for the Welch method.
+    total_secs: int, default: 100
+        The total number of seconds to use for the spectral density calculation.
+    only_ibl_specific_steps: bool, default: False
+        If True, only the IBL specific steps are run (i.e. skips calling `export_to_phy`)
+    compute_pc_features: bool, default: False
+        If True, pc features are computed
+    compute_amplitudes: bool, default: True
+        If True, waveforms amplitudes are computed
+    sparsity: ChannelSparsity or None, default: None
+        The sparsity object (currently only respected for phy part of the export)
+    copy_binary: bool, default: True
+        If True, the recording is copied and saved in the phy "output_folder"
+    remove_if_exists: bool, default: False
+        If True and "output_folder" exists, it is removed and overwritten
+    template_mode: str, default: "median"
+        Parameter "mode" to be given to WaveformExtractor.get_template()
+    dtype: dtype or None, default: None
+        Dtype to save binary data
+    verbose: bool, default: True
+        If True, output is verbose
+    use_relative_path : bool, default: False
+        If True and `copy_binary=True` saves the binary file `dat_path` in the `params.py` relative to `output_folder` (ie `dat_path=r"recording.dat"`). If `copy_binary=False`, then uses a path relative to the `output_folder`
+        If False, uses an absolute path in the `params.py` (ie `dat_path=r"path/to/the/recording.dat"`)


Suggested change

----------

analyzer: SortingAnalyzer

The sorting analyzer object.

output_folder: str | Path

The output folder where the phy template-gui files are saved

rms_win_length_sec: float, default: 3

The window length in seconds for the RMS calculation.

welch_win_length_samples: int, default: 1024

The window length in samples for the Welch method.

total_secs: int, default: 100

The total number of seconds to use for the spectral density calculation.

only_ibl_specific_steps: bool, default: False

If True, only the IBL specific steps are run (i.e. skips calling `export_to_phy`)

compute_pc_features: bool, default: False

If True, pc features are computed

compute_amplitudes: bool, default: True

If True, waveforms amplitudes are computed

sparsity: ChannelSparsity or None, default: None

The sparsity object (currently only respected for phy part of the export)

copy_binary: bool, default: True

If True, the recording is copied and saved in the phy "output_folder"

remove_if_exists: bool, default: False

If True and "output_folder" exists, it is removed and overwritten

template_mode: str, default: "median"

Parameter "mode" to be given to WaveformExtractor.get_template()

dtype: dtype or None, default: None

Dtype to save binary data

verbose: bool, default: True

If True, output is verbose

use_relative_path : bool, default: False

If True and `copy_binary=True` saves the binary file `dat_path` in the `params.py` relative to `output_folder` (ie `dat_path=r"recording.dat"`). If `copy_binary=False`, then uses a path relative to the `output_folder`

If False, uses an absolute path in the `params.py` (ie `dat_path=r"path/to/the/recording.dat"`)

----------

analyzer : SortingAnalyzer

The sorting analyzer object.

output_folder : str | Path

The output folder where the phy template-gui files are saved

rms_win_length_sec : float, default: 3

The window length in seconds for the RMS calculation.

welch_win_length_samples : int, default: 1024

The window length in samples for the Welch method.

total_secs : int, default: 100

The total number of seconds to use for the spectral density calculation.

only_ibl_specific_steps : bool, default: False

If True, only the IBL specific steps are run (i.e. skips calling `export_to_phy`)

compute_pc_features : bool, default: False

If True, pc features are computed

compute_amplitudes : bool, default: True

If True, waveforms amplitudes are computed

sparsity : ChannelSparsity or None, default: None

The sparsity object (currently only respected for phy part of the export)

copy_binary : bool, default: True

If True, the recording is copied and saved in the phy "output_folder"

remove_if_exists : bool, default: False

If True and "output_folder" exists, it is removed and overwritten

template_mode : str, default: "median"

Parameter "mode" to be given to WaveformExtractor.get_template()

dtype : dtype or None, default: None

Dtype to save binary data

verbose : bool, default: True

If True, output is verbose

use_relative_path : bool, default: False

If True and `copy_binary=True` saves the binary file `dat_path` in the `params.py` relative to `output_folder` (ie `dat_path=r"recording.dat"`). If `copy_binary=False`, then uses a path relative to the `output_folder`

If False, uses an absolute path in the `params.py` (ie `dat_path=r"path/to/the/recording.dat"`)

We are working on implementing numpydoc linting in the future and it requires there to be a space after the parameter.

zm711 · 2024-11-07T18:41:49Z

src/spikeinterface/exporters/to_ibl.py

+    try:
+        from scipy.signal import welch
+    except ImportError as e:
+        raise ImportError("Please install scipy to use the export_to_ibl function.") from e


I've never seen this before. How does this raise error from e work?

I would just import it in the function. The problem is that core tests will look in all files for tests, and script is not installed in core tests

samuelgarcia · 2024-11-08T09:17:43Z

src/spikeinterface/exporters/to_ibl.py

+        "fscale": fscale(welch_win_length_samples, 1 / fs_ap, one_sided=True),
+        "tscale": wingen.tscale(fs=fs_ap),
+    }
+    win["spectral_density"] = np.zeros((len(win["fscale"]), analyzer.recording.get_num_channels()))


I think we should have a proper function to compute the spectrum if you need it.
So we could have proper tests for this.

samuelgarcia · 2024-11-08T09:18:20Z

src/spikeinterface/exporters/to_ibl.py

+    total_samples_ap = int(np.min([fs_ap * total_secs, analyzer.recording.get_num_samples()]))
+
+    # the window generator will generates window indices
+    wingen = WindowGenerator(ns=total_samples_ap, nswin=rms_win_length_samples_ap, overlap=0)


we have already some function to generate window in spikeinterface.

samuelgarcia · 2024-11-08T09:25:24Z

Hi @jonahpearl
thanks for this contribution to spikeinetrface and making a bridge to ibl tools.

About to_ibl_utils.py I would identify function by function what the need and if there is an equivalent function in spikeinterface for it. Then I would implement then at the good place (core, preprocessing, postprocessing, ...) and also would make some tests function per function.

About the spectrum, I would implement it in a speparate file using the spikeinterface machinery on chunk parralelisation.

jonahpearl · 2024-11-08T13:53:00Z

Fair enough -- I don't have time to refactor this right now but I can work on it in the coming months.

alejoe91 and others added 30 commits March 8, 2024 18:01

release notes

d220c70

Update doc/whatisnew.rst

149b6a8

Co-authored-by: Garcia Samuel <[email protected]>

Merge pull request SpikeInterface#2558 from alejoe91/prepare_0.100.1_…

db49738

…release Prepare minor 0.100.1 release

fix ks4 GPU with container

6cbe4c5

Prepare 0.100.2

c7a020f

Add batch size to KS4 wrapper

af269ff

Add batch size PR and update date

59c19e6

Merge pull request SpikeInterface#2593 from alejoe91/ks4-batch-size-b…

f9ecdbf

…ug-fixes Add batch size to KS4 wrapper

Fix template similarity when there are multiple sizes of units

2eea2d5

[pre-commit.ci] auto fixes from pre-commit.com hooks

cff7886

for more information, see https://pre-commit.ci

Merge pull request SpikeInterface#2581 from alejoe91/prepare_0.100.2_…

e990d53

…release Prepare 0.100.2

Merge pull request SpikeInterface#2601 from chrishalcrow/fix_template…

9041249

…_similarity_diff_units Fix template similarity when there are multiple sizes of units

propagate SpikeInterface#2639 and SpikeInterface#2625

14b84cf

Propagate SpikeInterface#2637

9bb4b90

Bump up version

5c33455

Add release notes

f44a34f

Merge pull request SpikeInterface#2640 from alejoe91/prepare-0.100.3

22808ca

Prepare minor release 0.100.3

Extended zarr compression

40429b4

Port SpikeInterface#2643

05b5fda

Add release notes

27d1676

Remove extra .

7867b89

Merge pull request SpikeInterface#2644 from alejoe91/prepare-0.100.4

7d0e1da

Prepare 0.100.4

Open Ephys: Use discovered recording ids tp load sync timestamps

542886a

prepare release 0.100.5

3cddb50

Add CI on bug fixes

bb88a49

Merge pull request SpikeInterface#2677 from alejoe91/preprare_0.100.5

7c2656c

Preprare 0.100.5

port bug fixes from a couple PRs

1424060

fix has exceeding for bug fix

7ad9bbb

Merge pull request SpikeInterface#2720 from zm711/ks-ms

449e219

Port bug fix PRs from Main over to 0.100-bug-fixes

Prepare release 0.100.6

a5cb02f

alejoe91 and others added 8 commits April 30, 2024 14:30

Propagate SpikeInterface#2621

d8b7fdb

Merge pull request SpikeInterface#2776 from alejoe91/prepare-0.100.6

f21a7b7

Prepare release 0.100.6

working draft

d9cb70f

working

bf4e72a

Merge remote-tracking branch 'origin' into ibl_exporter

29e5b61

first draft ibl exporter

ac8b481

update analyzer extension checks

7135f28

[pre-commit.ci] auto fixes from pre-commit.com hooks

ca5d330

for more information, see https://pre-commit.ci

alejoe91 reviewed Nov 7, 2024

View reviewed changes

.github/workflows/core-test.yml Outdated Show resolved Hide resolved

alejoe91 requested changes Nov 7, 2024

View reviewed changes

alejoe91 reviewed Nov 7, 2024

View reviewed changes

jonahpearl and others added 5 commits November 7, 2024 13:00

Remove bug fixes branch

76f9284

Co-authored-by: Alessio Buccino <[email protected]>

remove changes

fdc801f

remove unused

a147110

import scipy inside func

5f0771b

[pre-commit.ci] auto fixes from pre-commit.com hooks

7beac9d

for more information, see https://pre-commit.ci

zm711 reviewed Nov 7, 2024

View reviewed changes

samuelgarcia reviewed Nov 8, 2024

View reviewed changes

alejoe91 added the exporters Related to exporters module label Nov 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ibl exporter #3522

Ibl exporter #3522

jonahpearl commented Nov 6, 2024

zm711 commented Nov 7, 2024

jonahpearl commented Nov 7, 2024

alejoe91 commented Nov 7, 2024

alejoe91 commented Nov 7, 2024

alejoe91 Nov 7, 2024

jonahpearl Nov 7, 2024

alejoe91 Nov 7, 2024

jonahpearl Nov 7, 2024

alejoe91 Nov 7, 2024

alejoe91 Nov 7, 2024

jonahpearl Nov 7, 2024

alejoe91 Nov 7, 2024

zm711 left a comment

zm711 Nov 7, 2024

zm711 Nov 7, 2024

alejoe91 Nov 7, 2024

samuelgarcia Nov 8, 2024

samuelgarcia Nov 8, 2024

samuelgarcia commented Nov 8, 2024

jonahpearl commented Nov 8, 2024

Ibl exporter #3522

Are you sure you want to change the base?

Ibl exporter #3522

Conversation

jonahpearl commented Nov 6, 2024

zm711 commented Nov 7, 2024

jonahpearl commented Nov 7, 2024

alejoe91 commented Nov 7, 2024

alejoe91 commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zm711 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelgarcia commented Nov 8, 2024

jonahpearl commented Nov 8, 2024