-
Notifications
You must be signed in to change notification settings - Fork 7
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
Batch inference issues when ROI close to image edge #45
Comments
thanks for the bug report. the video is very helpful! i wonder if the hiccups are caused by padding the images to get them to the required size. but this also brings up a concern for me. the images should be 224x224 after resizing. how is an image of 188x188 attempting to be batched with a 224x224 image, if all of the images should be resized to 224x224? is the 188x188 not undergoing the resizing for some reason? (but of course, a truncated patch should not be upscaled to 224x224, because the physical spacing will be wrong). also i am wondering why the image is a square (188x188) as opposed to a rectangle where one direction is shorter than the other direction. (although 188x188 could be the bottom right corner patch). if this isn't already done, we should decide how to pad the images. we could pad them with some constant color (like white). or we could mirror the patch. we could also exclude the patch if it extends past the boundaries of the slide. thoughts everyone? by the way @xalexalex - how do you record your videos? i would like to do something similar :) |
I thought the same thing, but it seems to take a bit too much time for a simple padding. I suspect something else is going on.
Good catch. Are we perhaps assuming that
I wil enumerate possible solutions from quick & hacky to thoughtful & expensive:
I would vote for 1. At most 2. But I think 1 will solve this problem quickly and nobody will ever complain. In my WSIs edge tiles are always uninformative and this error is simply an hindrance.
This is going to be very crude, but:
|
Quick update: the error also happens on the left and top edges. So solution (1) seems to be the quickest way to fix this, whereas (2) is not doable until we understand where the actual problem is. |
Does the WSInfer Python code have a strategy for this? To clarify: the QuPath implementation of inference is completely independent. It should agree with whatever is done in Python as much as possible for consistency, but it is difficult to guarantee identical results because some core operations might be implemented differently (e.g. the precise interpolation used when resizing tiles, which can make a big different). |
This PR addresses this by using zero-padding: #47 That is what should have been happening already... I just missed the bug because I was restricted to a batch size on 1 on my Mac (which is no longer a restriction). Other boundary criteria could be considered is WSInfer handles it differently in Python. I also added a comment where the tile resizing is applied: // For example, using the Python WSInfer 0.5.0 output for the image at
// https://github.com/qupath/qupath-docs/issues/89 (30619 tiles):
// BufferedImageTools Tumor prob Mean Absolute Difference: 0.0026328298250342763
// OpenCV Tumor prob Mean Absolute Difference: 0.07625036735485102 Basically, the method of interpolation makes a difference in how well the Python and QuPath implementations agree. I believe Python uses bilinear interpolation, but the results quoted above both use bilinear interpolation, just implemented differently and this is enough to cause disagreements. I think perfect agreement between Python and QuPath would be very difficult to achieve (and require some substantial changes), but this figure gives some idea of the difference. If I use something other than bilinear interpolation in QuPath, I see much larger disagreements. |
wsinfer python pads patches with 0. actually this is an implementation detail of opensldie and tiffslide (they will pad with 0 if the patch is at the edge of a slide). i should add tests to wsinfer python that makes sure this continues to happen with future versions.
i agree, and i believe it shouldn't be our goal to achieve perfect agreement. by the way, the bilinear resampling in wsinfer/python is performed by Pillow. |
@petebankhead I tested the current HEAD and unfortunately the issue isn't fixed for me. Could anyone else check? I get this error: Successful run without edge tiles:
Run that errors out on edge tile:
|
thanks @xalexalex it appears that these patches are not being resized for some reason... i'm not sure what would cause this.
|
This works for me with the zoo models on both Windows and Mac. @xalexalex can you specify which model you are using, or share the Two explanations I can think of:
@kaczmarj I notice that https://huggingface.co/kaczmarj/pancancer-lymphocytes-inceptionv4.tcga/blob/main/config.json contains a patch size of 100 but resizes to 299... is this intended? |
i realize it looks odd, but it is intended. i triple-checked the original implementation (https://github.com/ShahiraAbousamra/til_classification). |
This was indeed true. Adding a |
fantastic! glad it is working |
When the ROI for inference is close (or touches) the image edge, I get the following error:
I can reproduce it on both the OpenSlide and BioFormats backends. With the new version of the wsinfer extension (v0.3.0), if I set a batch size = 1, I don't get the error but whenever inference encounters a tile on the edge, it is markedly slower (I can see the hiccups in the progress bar: at first once every few batches [because only some batches will end with a bottom-edge tile] and then for each batch [since on the last column, each batch has an edge tile]).
With batch size >1, if after the error I look at the detections, I can see detections up to and excluding the first batch containing an edge tile.
example video showing the behavior with batch size 4 and then 1.
The text was updated successfully, but these errors were encountered: