SBU-BMI / wsinfer

🔥 🚀 Blazingly fast pipeline for patch-based classification in whole slide images
https://wsinfer.readthedocs.io
Apache License 2.0
57 stars 10 forks source link

RecursionError: maximum recursion depth exceeded in comparison #192

Closed jecaJeca closed 10 months ago

jecaJeca commented 1 year ago

Hi,

I am running the following command: wsinfer run --wsi-dir slides/ --results-dir results/ --model colorectal-tiatoolbox-resnet50.kather100kand I am getting the RecursionError: maximum recursion depth exceeded in comparison.

Finding patch coordinates...

Slide 1 of 1 (100.00%)
INFO:wsinfer.patchlib:Segmenting and patching slide /mypath/wsinfer_test/slides/mypath/image_openslide.tif
INFO:wsinfer.patchlib:Using prefix as slide ID: /mypath/image_openslide
INFO:wsinfer.patchlib:Slide has WxH (17600, 18944) and MPP=100.0
INFO:wsinfer.patchlib:Requested patch size of 224 px at 0.5 um/px
INFO:wsinfer.patchlib:Scaling patch size by 0.005 for patch coordinates at level 0 (MPP=100.0) | patch_spacing_um_px / mpp (0.5 / 100.0)
INFO:wsinfer.patchlib:Final patch size is 1
INFO:wsinfer.patchlib:Thumbnail has WxH (1903, 2048) and MPP=924.9277456647399
INFO:wsinfer.patchlib:Transformed minimum object size to 0 pixel area in thumbnail
INFO:wsinfer.patchlib:Transformed minimum hole size to 0 pixel area in thumbnail
INFO:wsinfer.patchlib.patch:Detected 1203 contours
INFO:wsinfer.patchlib.patch:Scaling contours to slide-level (level=0) coordinate space by multiplying by (9.248554913294798, 9.25)
ERROR:wsinfer.patchlib:Failed to segment and patch slide
/mypath/wsinfer_test/slides//mypath/image_openslide.tif
Traceback (most recent call last):
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/__init__.py", line 357, in segment_and_patch_directory_of_slides
    segment_and_patch_one_slide(
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/__init__.py", line 161, in segment_and_patch_one_slide
    polygon, contours, hierarchy = get_multipolygon_from_binary_arr(
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/patch.py", line 100, in get_multipolygon_from_binary_arr
    polygon = merge_polygons(MultiPolygon(), 0, True)
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/patch.py", line 95, in merge_polygons
    polygon = merge_polygons(polygon, next_idx, add)
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/patch.py", line 95, in merge_polygons
    polygon = merge_polygons(polygon, next_idx, add)
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/patch.py", line 95, in merge_polygons
    polygon = merge_polygons(polygon, next_idx, add)
  [Previous line repeated 977 more times]
  File "/mypath/lib/python3.9/site-packages/wsinfer/patchlib/patch.py", line 75, in merge_polygons
    if not new_poly.is_valid:
  File "/mypath/lib/python3.9/site-packages/shapely/geometry/base.py", line 638, in is_valid
    return bool(shapely.is_valid(self))
  File "/mypath/lib/python3.9/site-packages/shapely/decorators.py", line 77, in wrapped
    return func(*args, **kwargs)
  File "/mypath/lib/python3.9/site-packages/shapely/predicates.py", line 401, in is_valid
    warnings.simplefilter("ignore")
  File "/mypath/lib/python3.9/warnings.py", line 179, in simplefilter
    _add_filter(action, None, category, None, lineno, append=append)
  File "/mypath/lib/python3.9/warnings.py", line 186, in _add_filter
    filters.remove(item)
RecursionError: maximum recursion depth exceeded in comparison

Can you, please, help me resolve this?

Thanks, Jelica

swaradgat19 commented 1 year ago

Hi @jecaJeca. What slide images are you using? I tried the same command on a random slide image and it did work.

It may be possible that the system you're working on has a lower recursion limit than what is required for WSInfer to run. You can try setting a custom recursion limit in Python in the following ways:

Print out the current recursion limit by executing the following code:

import sys
print(sys.getrecursionlimit())

You can change the recursion limit by executing the following:

sys.setrecursionlimit(1500)

Caution: Gradually increase the recursion limit

kaczmarj commented 1 year ago

Hi @jecaJeca - try raising the recursion limit to 1500 like @swaradgat19 suggests and let us know if that solves the issue. i explain why i think this is happening below.

by the way, according to the logs you posted, wsinfer thinks the slide has a physical resolution of 100.0 microns per pixel. Is that accurate?

@swaradgat19 - thanks for the help. the error is being thrown in wsinfer.patchlib.patch.merge_polygons

https://github.com/SBU-BMI/wsinfer/blob/123d7c1aaa8bf72b00e857a090db6385db069720/wsinfer/patchlib/patch.py#L59-L103

that's a recursive function and recurses at most the number of contours detected. in the case of this issue, there are 1203 contours, which is greater than python's default recursion limit of 1000. we might want to include a fix to temporarily raise the recursion limit to the number of contours if it exceeds the recursion limit. an elegant solution imho would be to create a context manager that temporarily changes the recursion limit, similar to https://stackoverflow.com/a/50120316

swaradgat19 commented 1 year ago

@kaczmarj That makes sense. So we should just define a class and simply use this code inside the merge_polygons function:

with recursionlimit(len(contour)):
    # pass it to Polygon function
    new_poly = Polygon(contour)
    # rest of the code

I'm not entirely familiar with this file, do let me know if we can do that in a better way

kaczmarj commented 1 year ago

@swaradgat19 -i am thinking we use that recursion limit context manager on this line:

https://github.com/SBU-BMI/wsinfer/blob/d5057345ef0948a3f53488a03331d3b061b36953/wsinfer/patchlib/patch.py#L99-L100

here's an implementation of a temporary recursion changer using a context manager.

from contextlib import contextmanager
import sys

@contextmanager
def temporary_recursion_limit(limit: int):
    old_limit = sys.getrecursionlimit()
    try:
        yield sys.setrecursionlimit(limit)
    finally:
        sys.setrecursionlimit(old_limit)

and we can use this like this:

with temporary_recursion_limit(len(contours))
    # Call the function with an initial empty polygon and start from contour 0 
    polygon = merge_polygons(MultiPolygon(), 0, True) 
swaradgat19 commented 10 months ago

I was looking into this issue. I have implemented this inside a try-except, since making the polygon inside a with will always reset the recursion limit.

try: 
    polygon = merge_polygons(MultiPolygon(), 0, True) 
except: 
    with temporary_recursion_limit(len(contours))
        # Call the function with an initial empty polygon and start from contour 0 
        polygon = merge_polygons(MultiPolygon(), 0, True) 

I observed that the tests in tests_all.py aren't passing. I reverted the changes and ran it. It still gives me 5 failed test cases:

E       AssertionError: assert 1 == 0
E        +  where 1 = <Result TorchRuntimeError('Failed running call_module fn(*(FakeTensor(..., device=\'cuda:0\', size=(s0, 3, s1, s2)),),...tion and fall back to eager by setting:\n    import torch._dynamo\n    torch._dynamo.config.suppress_errors = True\n')>.exit_code
====================================================== short test summary info =======================================================
FAILED tests/test_all.py::test_cli_run_with_registered_models[tiffslide-True-breast-tumor-resnet34.tcga-brca] - AssertionError: assert 1 == 0
FAILED tests/test_all.py::test_cli_run_with_registered_models[tiffslide-True-lung-tumor-resnet34.tcga-luad] - AssertionError: assert 1 == 0
FAILED tests/test_all.py::test_cli_run_with_registered_models[tiffslide-True-pancancer-lymphocytes-inceptionv4.tcga] - AssertionError: assert 1 == 0
FAILED tests/test_all.py::test_cli_run_with_registered_models[tiffslide-True-pancreas-tumor-preactresnet34.tcga-paad] - AssertionError: assert 1 == 0
FAILED tests/test_all.py::test_cli_run_with_registered_models[tiffslide-True-prostate-tumor-resnet34.tcga-prad] - AssertionError: assert 1 == 0
======================================== 5 failed, 18 passed, 15 skipped, 1 xfailed in 57.75s =============

I have pulled the latest changes too. It is failing in the test_cli_run_with_registered_models. Is it working on your end?

swaradgat19 commented 10 months ago

My bad. I pushed changes on my repo and it passed all test cases. It is not running locally, though. I'll look into it.

kaczmarj commented 10 months ago

instead of the try/except, let's just use

with temporary_recursion_limit(len(contours))
    # Call the function with an initial empty polygon and start from contour 0 
    polygon = merge_polygons(MultiPolygon(), 0, True) 

regarding the test failures, make sure you pull the most recent state of the main branch.

another note, don't use a bare except statement, because that will catch KeyboardInterrupt exceptions too, so you wouldn't be able to control+C to quit the program at that point. use except Exception at the very least.

swaradgat19 commented 10 months ago

I used try except because, if we only use with, it would change the recursion limit each time we make a new polygon right? I was attempting to change the recursion limit only when polygon = merge_polygons(MultiPolygon(), 0, True) fails, ie, the len(contours) exceeds the recursion limit. Although do correct me if my understanding is wrong.

kaczmarj commented 10 months ago

your understanding is correct. i would avoid running merge_polygons twice. how about we do this: we temporarily set the recursion limit to

temp_limit = max(sys.getrecursionlimit(), len(contours))
with temporary_recursion_limit(temp_limit)
    # Call the function with an initial empty polygon and start from contour 0 
    polygon = merge_polygons(MultiPolygon(), 0, True)