Added detect subvolume crop support

[parikshit14]

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

There already existed the feature to select the range of planes to run detect on. But with the addition of giving the range of height and width, gives us the ability to run detect on a cropped area within the plane.
Example:
signal_array -> 3D array of size (100, 50, 20) [No. of Planes, Height, Width] and given start-plane = 1 and end-plane = 6
instead of running on the entire range of 5 planes each of size 50*20, we can now run it on specific area according to given height width range.

What is this PR

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?
Adds support for cropping a subvolume, hence we can now control within the plane, the height and width to run detect on.

What does this PR do?
Added parameters to detect's main() to crop the height and width of the selected planes.

References

Please reference any existing issues/PRs that relate to this PR.
issue #468

How has this PR been tested?

Yes

Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.
Added unit tests to check the additional parameters influence

Is this a breaking change?

Might not, but I need to check napari plugins to add this functionality there as well.

If this PR breaks any existing functionality, please explain how and why.
No

Does this PR require an update to the documentation?

If any features have changed, or have been added. Please explain how the documentation has been updated (and link to the associated PR). See here for details.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality (unit & integration)
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[adamltyson]

Hi @parikshit14. The main use case for #468 is to enable this in the napari plugin. Would you be able to also add that functionality?

[parikshit14]

Hi @adamltyson, It's a work in progress. Sorry, I forgot to open it as a draft . Moving.....

[parikshit14]

Hi @adamltyson, I have updated the Napari plugin to support detection on a given subvolume. Moving from draft.

[adamltyson]

Hi @parikshit14, could you explain what the changes in the cube generator are for? In theory, changing the volume for detection shouldn't change the classification step.

[parikshit14]

Yup this is indeed strange, actually it is nowhere used in the classification step. However, since the function get_cube_depth_min_max was defined here, to maintain similar flow, I defined the functions get_cube_width_min_max , get_cube_height_min_max in the same file. If you suggest, I can move all these three functions out from classify/cube_generator and into napari/detect/detect.py where it is actually used.

[adamltyson]

Ah, I understand. I think it's ok to leave it here.

[adamltyson]

This PR seems to be causing our brainmapper tests to fail. Do you know why? Ideally this PR shouldn't be a breaking change.

[parikshit14]

This PR seems to be causing our brainmapper tests to fail. Do you know why? Ideally this PR shouldn't be a breaking change.

Before creating the PR, I ran all the tests present in the unit and integration test folders which passed. So I missed what could go wrong, looking into it.....

[adamltyson]

Sorry I should have mentioned. We have additional tests to make sure that changes don't break other software.

[parikshit14]

Hi @adamltyson I have made changes in the workflows, this will fix the current error. There also things might break (mentioned in brainglobe/brainglobe-workflows#145). However I have tested things locally, and tests are running without error.

[adamltyson]

Hi @parikshit14, I tried this out in the napari plugin, following the cell detection tutorial.

Setting limits on the height/width doesn't seem to change the cells that are detected. I set the end width and end height to 250 (roughly in the middle of the image), but cells are detected across the image.

[parikshit14]

HI @adamltyson , somewhere in reordering, I messed up. Thanks for pointing that out. Please check, tested locally with workflow test, should pass now.

[adamltyson]

Hi @parikshit14, I tested this in napari, firstly with the sample data supplied by the plugin. This looked to work ok. I then tried it with some data loaded via dask (a common workflow with cellfinder), and the results weren't correct. There were no cells detected in the correct place, but many rejected artefacts found outside the brain (but in a cluster suggestive of real data rather than noise).

Do you have any idea what's going on?

[parikshit14]

Hi @adamltyson, sorry for the trouble and so many unsuccessful attempts you had to deal with.

Can you maybe highlight how to recreate the results you mentioned? I am assuming this ref

[adamltyson]

Hi @parikshit14, I used the napari plugin to load a directory of 2D tiffs. I think you should be able to use any data for this, but if you can't replicate, let me know and I will try to pin it down.

[adamltyson]

Also, no problem at all, thanks for contributing!

[matham]

Hi @parikshit14, I tested this in napari, firstly with the sample data supplied by the plugin. This looked to work ok. I then tried it with some data loaded via dask (a common workflow with cellfinder), and the results weren't correct. There were no cells detected in the correct place, but many rejected artefacts found outside the brain (but in a cluster suggestive of real data rather than noise).

There are two potential issues responsible for this, that should probably be addressed for this feature anyway.

1. The way the algorithm works, during 2d filtering, it tiles each plane (

   cellfinder/cellfinder/core/detect/filters/plane/plane_filter.py

   Line 126 in 091345f

   inside_brain_tiles = self.tile_walker.get_bright_tiles(planes)

   ) and uses a corner tile of the plane to decide a threshold that determines whether each tile is inside our outside the brain. If a tiles intensity is above twice the standard deviation of the corner it's inside. If it's considered outside, it is excluded from cell counting (

   cellfinder/cellfinder/core/detect/filters/volume/ball_filter.py

   Line 238 in 091345f

   def append(

   ).

   The obvious issue is that if we're taking a sub-area of the plane, the corner is much more likely to fall within the tissue. So we'd be excluding tiles incorrectly, because the background intensity would be wrong. While this is a problem in general, in case the sample doesn't have an "outside" region, it's much more likely here.

   So I think that would require a way to turn off this tiling feature

2. More directly relevant to this PR, if you're taking a sub-area of the plane, the position kicked out by the cell detector will be relative to the corner of the sub-area. I.e. (0, 0, 0) is the corner of the sub-area. But when you return it from main, it'll be considered relative to the corner of the overall sample. So you need to properly add the offset to each position in the cell detector. And everywhere else this may effect this.

Also, there should probably be tests that test exactly the second point. Where you know there's a cell at some coordinate. The test would pass in a sub-volume and check that the output coordinates are unchanged.

[adamltyson]

The obvious issue is that if we're taking a sub-area of the plane, the corner is much more likely to fall within the tissue. So we'd be excluding tiles incorrectly, because the background intensity would be wrong. While this is a problem in general, in case the sample doesn't have an "outside" region, it's much more likely here.

So I think that would require a way to turn off this tiling feature

We have an open issue for this (#292). It's generally a problem with the current cellfinder implementation, but I think for now can be delt with seperately to this PR.