JaneliaSciComp / bigstream

Tools for distributed alignment of massive images
BSD 3-Clause "New" or "Revised" License
74 stars 21 forks source link

check mov_mask for foreground #42

Closed orena1 closed 4 months ago

orena1 commented 5 months ago

From what I understand blocks which are all in the mask should not be considered for the registration, I think checking the mov_mask is missing.

GFleishman commented 4 months ago

Hi @orena1! Thanks for another BigStream PR!

This is a good thought, but actually cannot be merged. There is a reason we only check the fixed image mask for determining which blocks to process. Recall that the fixed and moving image do not have to be on the same voxel grid. For example, you might have a fixed image shape of (1000, 2000, 3000) and spacing of (1.0, 0.5, 0.5) and a moving image shape of (500, 750, 1000) and spacing of (1.5, 2.0, 2.0). It's still perfectly ok to register these two images.

Now, when we carve the image into blocks we must choose which voxel grid upon which to define our blocks, it cannot be consistently defined on both voxel grids at the same time when we account for the possibility of the images having different shapes. We definitely want to choose the fixed image voxel grid to define our blocks, since the final deform will also be defined on this voxel grid. Simply looking at the same voxel coordinates in the moving image/mask does not account for the possibility that the grids could be different. It also does not account for any image origins or initial transforms given in the static_transform_list.

Alternatively, what would make sense, is to check the fixed mask in the block as we do now, but then determine exactly which region this block corresponds to in the moving voxel grid - after accounting for the different voxel spacings, possibly different image origins, and any initial transforms given. Then we could check the moving image mask in that mapped region. However this is a very expensive computation to do up front in the main process for every block and the feature is not worth the expensive computation. We do have to do this once we have mapped all the blocks to separate workers, see here, but that is done on the workers, so it's in parallel and isn't a bottle neck for the overall computation.

Now that I think about this however, a better implementation would be to do this moving mask foreground check in the distributed function, instead of up front. This would avoid trying to align blocks which have fixed data foreground to blocks that have no moving image foreground - which would likely result in a distortion. Let me know if you're following me here, because I think this is a good idea worth discussing, it just needs to be in a different spot.

GFleishman commented 4 months ago

The idea is that the moving mask foreground check would occur just after this line. If there is foreground, we just proceed with the alignment. If there is no foreground I think we would need to create a dummy transform result - a vector field of the correct shape with all zeros - and then proceed with that as if it had actually been computed. Basically we would just be skipping the call to the alignment pipeline. That would save time, and it would also possibly prevent distortions in areas where there is fixed foreground but no moving foreground.

orena1 commented 4 months ago

Thanks a lot @GFleishman , I understand the problem.

My work involves registering 2d-slices to 3d, the 2d-slices are extended to 3D stacks, but many of the pixels in that fake-3D stack are 0s. This result with trying to register a real-3D-block to a block with is black and than we get to this error: I assume I am lucky as when I move the 2D slice to fake-3D I transform the spacing to be identical to the 3D, so my solution works, but indeed it will not be general enough.

I must say that I do not really understand how the spacing works, I understand the intuition but do not really understand how you overcome it, so I do not know how to implement your suggested solutions.

Thanks

GFleishman commented 4 months ago

Hi again - thanks for describing your application. Lots of others have asked me about 2D to 3D registration so I'm really excited that someone is using BigStream to make that possible. Now that I understand your problem, there are two new feature proposals I can suggest that might make your workflow easier. I may have time to implement one of these myself soon.

Feature proposal 1: Add virtual masking options

Currently, fix_mask and mov_mask must either be None or a numpy array containing ones and zeros. That is, you have to explicitly create your own mask outside of bigstream and then provide it. I would consider allowing fix_mask and mov_mask to also be a tuple of arbitrary length containing ints or floats. In such a case, the function would consider any value in that tuple to be background, and create masks internally where appropriate. Going one step further I could allow fix_mask and mov_mask to be a function which takes an array as input and returns an array of bools. In that case, bigstream would apply that function internally at the appropriate times to create masks.

For your application, you could then just do mov_mask=(0,) to mask out all your padded data. Internally this would be much easier to work with than a large array equal in size to the image itself.

Feature proposal 2: Add image origins to the distributed_piecewise_alignment_pipeline

Right now it sounds like when you convert from 2D to 3D you are simply padding planes of zeros on top of and below the real data. You would do this such that the final shape of the padded "faked 3D" image is the same as the fixed image shape, and you would put the correct number of planes above and below such that the position of the real data plane was as close as possible to where you think it needs to register into the fixed 3D volume. Is that correct?

If you were doing this with the in-memory non-distributed function I would suggest that you use a mov_origin. All the alignment functions in align.py take this argument. Here it is for the alignment_pipeline. The documentation is obviously pretty sparse so I don't blame anyone for not understanding how this could help them. Basically the origins let you specify how the fixed and moving image voxel grids should be positioned relative to one another.

So, for your application, instead of padding with a really large number of planes above and below the real data plane, you would pad with just a few planes above and below, and then define a mov_origin that was equal to the offset of the moving data (0, 0, 0) voxel from the fixed data (0, 0, 0) voxel. Origins are in physical units, so you would need to take the voxel spacings into account.

However - origins are currently not available for the distributed pipelines because they complicate the blocking process. However I think this is solvable and I may be willing to work on it in the future.

GFleishman commented 4 months ago

Also @orena1, I noticed that you have not clicked the star button for the bigstream repository. If you are finding it useful, and want to motivate us to add features and provide support, can you please do so? The best reason to continue building open source software is because you know it is making an impact. Simple metrics like the star count help me demonstrate impact.

orena1 commented 4 months ago

Hi @GFleishman , so the 2D -to- 3D stack is a plane, but it is manually warpped with bigwarp, the results is a warpped 2D plane inside a cube, usually the some part of the plane (albit small) is visible in each Z-plane of the fake stack.

I think the first solution that you suggested is more easy to understand and use as a user. Thanks a lot!

My current use-case currently is to finetune the bigwarp registration using deform registration with bigstream.

GFleishman commented 4 months ago

Hi again! Thanks for following through and starring the repository, I'm really grateful!

That's great that you're combining BigWarp and BigStream - that is a really powerful workflow that should be able to handle almost any image registration problem. You may not want to get into this too much, but it is possible to take the transform from BigWarp and use it with BigStream. Here's a post I wrote recently helping someone else do just that: https://forum.image.sc/t/combining-bigwarp-and-bigstream/95663/3

Ok - I'm going to close this PR without merge, but again thanks so much for offering it and sparking a good discussion. I'm going to submit a feature-request to the issues section of the repository describing the two features above and I'll note that you're more interested in feature (1), which I agree is more easy to use right away.