Skip to content

Parallelize coherence-based bad channel detection - #4750

Open
JESUSROYETH wants to merge 1 commit into
SpikeInterface:mainfrom
JESUSROYETH:radar/fix-spikeinterface-2869
Open

Parallelize coherence-based bad channel detection#4750
JESUSROYETH wants to merge 1 commit into
SpikeInterface:mainfrom
JESUSROYETH:radar/fix-spikeinterface-2869

Conversation

@JESUSROYETH

Copy link
Copy Markdown

Description

The default coherence+psd method first materialises 100 scaled chunks and then processes them in serial. On a 384-channel recording, profiling shows that the Welch and median computations dominate the call, while extracting the random traces takes a small part of the total time.

This change sends the same random slices through TimeSeriesChunkExecutor. The default stays at n_jobs=1, but the chunks are streamed instead of kept together in memory. A final job_kwargs parameter lets callers select a thread or process pool without changing positional arguments.

I measured the default 100 × 0.3 s chunk path on an Intel Core i9-13900HX, with numerical thread pools pinned to one thread. The generated row uses the 30 s / 384-channel shape reported in the issue. The second row uses real 384-channel SpikeGLX traces from the public Noise4Sam fixture; the public file is short, it was repeated in memory to cover the same chunk path.

Workload Before After (n_jobs=8, processes) Improvement
Generated 30 s recording 8.454 s (8.384–8.479) 2.193 s (2.118–2.236) 3.86x
Noise4Sam real traces 12.836 s (12.749–12.964) 2.984 s (2.973–3.014) 4.30x

The channel ids and labels matched exactly between both implementations in each run. On the serial real-trace path, three fresh-process peak RSS measurements gave a median of 1,688,604 KiB before (range 1,688,500-1,689,352) and 353,452 KiB after (range 352,984-354,896), around 79% less. This comes from processing one random slice at a time instead of retaining the scaled chunks.

Validation:

  • pytest -q src/spikeinterface/preprocessing/tests/test_detect_bad_channels.py: 11 passed.
  • The regression tests cover a thread pool, a spawned process pool, and a two-segment recording.
  • A second spawned-process test uses a recording that is not pre-marked as filtered, so the
    highpass_filter wrapper detect_bad_channels builds internally also gets exercised across a process boundary; it matches the serial result on the same data.
  • Both new tests fail on the matching main revision and pass with this patch.
  • black and the trailing-whitespace/end-of-file checks are clean.

Fixes #2869.

@alejoe91 alejoe91 added preprocessing Related to preprocessing module performance Performance issues/improvements labels Sep 3, 2026
Comment on lines +291 to +297
if job_kwargs is None:
job_kwargs = {"progress_bar": False}
job_kwargs = fix_job_kwargs(job_kwargs)
executor_job_kwargs = {
key: job_kwargs[key]
for key in ("pool_engine", "n_jobs", "progress_bar", "mp_context", "max_threads_per_worker")
}

@alejoe91 alejoe91 Sep 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if job_kwargs is None:
job_kwargs = {"progress_bar": False}
job_kwargs = fix_job_kwargs(job_kwargs)
executor_job_kwargs = {
key: job_kwargs[key]
for key in ("pool_engine", "n_jobs", "progress_bar", "mp_context", "max_threads_per_worker")
}
job_kwargs = {} if job_kwargs is None else job_kwargs
job_kwargs = fix_job_kwargs(job_kwargs)

@JESUSROYETH why isn't this enough to propagate to the TimeSeriesChunkExecutor?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from spikeinterface.core.job_tools import fix_job_kwargs

job_kwargs = dict(n_jobs=4)

print(fix_job_kwargs(job_kwargs)

>>> {'pool_engine': 'process',
 'n_jobs': 4,
 'chunk_duration': '1s',
 'progress_bar': True,
 'mp_context': None,
 'max_threads_per_worker': 1}

@alejoe91 alejoe91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JESUSROYETH

I just have one comment but this looks great! :)

handle_returns=True,
chunk_size=random_chunk_kwargs["chunk_size"],
job_name="detect_bad_channels",
**executor_job_kwargs,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**executor_job_kwargs,
**job_kwargs,

See https://github.com/SpikeInterface/spikeinterface/pull/4750/changes#r3923673650

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance issues/improvements preprocessing Related to preprocessing module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

detect_bad_channels is really slow

2 participants