brainglobe / cellfinder-core

Standalone cellfinder cell detection algorithm
https://brainglobe.info/documentation/cellfinder/index.html
BSD 3-Clause "New" or "Revised" License
19 stars 16 forks source link

ensure cellfinder-core can run on single cpu #198

Closed alessandrofelder closed 1 year ago

alessandrofelder commented 1 year ago

Description

What is this PR

Why is this PR needed? It's currently possible for cellfinder-core to spawn a threadpool with 0 processes. See https://github.com/brainglobe/cellfinder-napari/issues/157

What does this PR do? Ensures n_ball_procs is always at least 1, and therefore a thread pool with 0 processes is never spawned.

References

Closes brainglobe/cellfinder-core#197 and should get us most of the way to https://github.com/brainglobe/cellfinder-napari/issues/157

How has this PR been tested?

Added a test parameter to check this edge case

Is this a breaking change?

No

Checklist:

alessandrofelder commented 1 year ago

Still need to test this on a real-life dataset (first thing tomorrow) - will confirm here.

adamltyson commented 1 year ago

Should we add tests for cellfinder-napari in the same way we do for cellfinder? Or just wait until we merge cellfinder-core & cellfinder-napari?

alessandrofelder commented 1 year ago

Should we add tests for cellfinder-napari in the same way we do for cellfinder? Or just wait until we merge cellfinder-core & cellfinder-napari?

I think it's not much work to add the additional tests to this workflow, and takes pressure off the merging repo timeline, so a good idea? Opened brainglobe/cellfinder#277

alessandrofelder commented 1 year ago

Confirming this ran on real-life large test data on 1 CPU and gave the same result as with 17 CPUs (took ~2:15 h instead of 1:05 h)