Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for patch commander #68

Closed

Conversation

JoeySpronck
Copy link
Collaborator

Fix for patch commander skipping patches that should be sampled on the borders. Also adds new optional functionality to not sample patches that do not have mask in the write-shape. This write shape is the shape to which the user will center crop the patches provided by the iterator for sliding window configs with overlap, where you crop off the overlapping borders

…e borders. Also adds new optional functionality to not sample patches that do not have mask in the write-shape. This write shape is the shape to which the user will center crop the patches provided by the iterator for sliding window configs with overlap, where you crop off the overlapping borders
@JoeySpronck
Copy link
Collaborator Author

@martvanrijthoven Important fix here, happy to see it merged so I can rebuild my dockers and rerun inference.

@martvanrijthoven
Copy link
Collaborator

Hey Joey, thanks!

One test failed:
FAILED tests/patch_iterator_test.py::test_patch_iterator - assert 1026 == 900

Could you check that out? Please let me know if you have any questions

@JoeySpronck
Copy link
Collaborator Author

JoeySpronck commented Oct 17, 2024

The issue

BrokenPipeError: [Errno 32] Broken pipe

    def test_patch_iterator():
        patch_configuration = PatchConfiguration(patch_shape=(1024,1024,3),
                                                spacings=(0.5,),
                                                overlap=(0,0),
                                                offset=(0,0),
                                                center=False)
    
        with create_patch_iterator(image_path='/tmp/TCGA-21-5784-01Z-00-DX1.tif',
                                patch_configuration=patch_configuration,
                                cpus=4,
                                backend='openslide') as patch_iterator:
    
            print(f"Number of patches {len(patch_iterator)}\n")
>           assert len(patch_iterator) == 900
E           assert 1026 == 900
E            +  where 1026 = len(<wholeslidedata.iterators.patchiterator.PatchBufferIterator object at 0x7f884fff3850>)

tests/patch_iterator_test.py:22: AssertionError

Comment

I think this is actually the issue Im trying to resolve, but I could make asome improvements.

The problem was that the iterator skipped tiles that should be sampled. For more complex configs many things could go wrong with the current setup.

What I solved

  1. What happened now for example: the double for loop on rows and columns stopped once the border of the image was reached, but with center=True the next row or column (for example coordinates that exeed the dimensions only 1 pixel) would also still sample parts within the dimensions of the image. Therefore the max row and col values should be increased with half the patch_size if center=True. I also added the offset to the max value, but that im not 100% sure off.

  2. Another issue though is that with a positive offset, you may skip patches that should be sampled left and above the first coordinate (with a negative offset likely the opposite). To be sure that multiple types of complex configs work as intended, I added a full additional patch on all sides (left, right, top, bottom). This will result in more patches in the messages, of which many will be ignored (not sampled) because the mask will be empty.

About this specific test

  • The increased x and y max with 1) the offset and 2) 1/2 patch shape if center is true, didnt happen in this test because they are conditional on those variables.
  • However, the additional step on all sides did happen
  • Becasue there is no mask in the test the unnecessary patches are not filtered out.

Proposal

I will make the addition of the border conditional on whether there is offset or overlap, otherwise the additional border will be skipped.

Happy to hear your feedback

@JoeySpronck
Copy link
Collaborator Author

@martvanrijthoven tests now successful

@JoeySpronck
Copy link
Collaborator Author

quick heads up that im completely reworking it, the full loop should be reworked instead of adjusted

@JoeySpronck
Copy link
Collaborator Author

Ill close this PR and open a new one for the rework

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants