search

ANTsX / ANTsPy

A fast medical imaging analysis library in Python with algorithms for registration, segmentation, and more.
https://antspyx.readthedocs.io
Apache License 2.0
629 stars 161 forks source link

resample_image_to_target / images_to_matrix #641

Closed cookpa closed 4 months ago

cookpa commented 4 months ago

Tracking down the Windows access exceptions

2024-05-18T17:03:16.9684885Z D:\a\ANTsPy\ANTsPy\tests\test_core_ants_image.py ....................... [ 12%]
2024-05-18T17:03:20.1751164Z .................                                                        [ 20%]
2024-05-18T17:03:24.2830571Z Windows fatal exception: Windows fatal exception: access violationaccess violation
2024-05-18T17:03:24.2831769Z 
2024-05-18T17:03:24.2831780Z 
2024-05-18T17:03:24.2831822Z 
2024-05-18T17:03:24.2832116Z Thread 0x00001770 (most recent call first):
2024-05-18T17:03:24.2835740Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\ants\ops\resample_image.py", line 172 in resample_image_to_target
2024-05-18T17:03:24.2839226Z   File "Windows fatal exception: C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\ants\utils\matrix_image.py", line 151Windows fatal exception: access violationaccess violation
2024-05-18T17:03:24.2842390Z 
2024-05-18T17:03:24.2842399Z 
2024-05-18T17:03:24.2842406Z 
2024-05-18T17:03:24.2842588Z  in listfunc
2024-05-18T17:03:24.2844421Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\ants\utils\matrix_image.py", line 169 in images_to_matrix
2024-05-18T17:03:24.2846540Z   File "D:\a\ANTsPy\ANTsPy\tests\test_core_ants_image_io.py", line 189 in test_images_to_matrix
2024-05-18T17:03:24.2848802Z   File "C:\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.0\tools\Lib\unittest\case.py", line 589 in _callTestMethod
2024-05-18T17:03:24.2851367Z   File "C:\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.0\tools\Lib\unittest\case.py", line 634 in run
2024-05-18T17:03:24.2853989Z   File "C:\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.0\tools\Lib\unittest\case.py", line 690 in __call__
2024-05-18T17:03:24.2856664Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\unittest.py", line 343 in runtest
2024-05-18T17:03:24.2859778Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\runner.py", line 173 in pytest_runtest_call
2024-05-18T17:03:24.2862675Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
2024-05-18T17:03:24.2865737Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
2024-05-18T17:03:24.2868403Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
2024-05-18T17:03:24.2871126Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\runner.py", line 241 in <lambda>
2024-05-18T17:03:24.2873846Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\runner.py", line 341 in from_call
2024-05-18T17:03:24.2876487Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\runner.py", line 240 in call_and_report
2024-05-18T17:03:24.2879300Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\runner.py", line 135 in runtestprotocol
2024-05-18T17:03:24.2882121Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\runner.py", line 116 in pytest_runtest_protocol
2024-05-18T17:03:24.2884160Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
2024-05-18T17:03:24.2886107Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
2024-05-18T17:03:24.2887995Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
2024-05-18T17:03:24.2889886Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\main.py", line 364 in pytest_runtestloop
2024-05-18T17:03:24.2891789Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
2024-05-18T17:03:24.2893629Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
2024-05-18T17:03:24.2895603Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
2024-05-18T17:03:24.2897053Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\main.py", line 339 in _main
2024-05-18T17:03:24.2899144Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\main.py", line 285 in wrap_session
2024-05-18T17:03:24.2900705Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\main.py", line 332 in pytest_cmdline_main
2024-05-18T17:03:24.2902406Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
2024-05-18T17:03:24.2904214Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
2024-05-18T17:03:24.2905734Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
2024-05-18T17:03:24.2907457Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\config\__init__.py", line 178 in main
2024-05-18T17:03:24.2909265Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Lib\site-packages\_pytest\config\__init__.py", line 206 in console_main
2024-05-18T17:03:24.2910806Z   File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-fp52okg3\cp312-win_amd64\venv-test\Scripts\pytest.exe\__main__.py", line 7 in <module>
2024-05-18T17:03:24.2911655Z   File "<frozen runpy>", line 88 in _run_code
2024-05-18T17:03:24.2912147Z   File "<frozen runpy>", line 198 in _run_module_as_main

This line is implicated

https://github.com/ANTsX/ANTsPy/blob/cd2a4acafc9fa38ea435de8fb9160522f024f242/ants/utils/matrix_image.py#L151

This call looks wrong to me, it doesn't match the function

https://github.com/ANTsX/ANTsPy/blob/cd2a4acafc9fa38ea435de8fb9160522f024f242/ants/ops/resample_image.py#L79

also within resample_image.py, I don't think this will work

https://github.com/ANTsX/ANTsPy/blob/cd2a4acafc9fa38ea435de8fb9160522f024f242/ants/ops/resample_image.py#L157

So why doesn't this fail everywhere? It's a problem I've come across before, where library functions are called and passed references to clones of the input. If the library function doesn't write to its output image for any reason, the cloned input is returned unmodified. I'd appreciate any ideas of how to address this problem systematically, because I think it can be the source of many quiet bugs.

ncullen93 commented 4 months ago

That call is actually OK, I think. If interp_type is an int it gets handled:

    interpolator_oldoptions = ("linear", "nearestNeighbor", "gaussian", "cosineWindowedSinc", "bSpline")
    if isinstance(interp_type, int):
        interpolator = interpolator_oldoptions[interp_type]

Otherwise, I agree that there should be a way to catch errors on library calls. Perhaps when calling get_lib_fn we actually return a call to the lib fn that is wrapped in a try-catch block.

ncullen93 commented 4 months ago

But yes that test looks incorrect:

import ants
import numpy as np

img = ants.image_read(ants.get_data('r16'))
imglist = [img.clone(),img.clone(),img.clone()]

s = [65]*img.dimension
mask2 = ants.from_numpy(np.random.randn(*s))
mask2 = mask2 > mask2.mean()
imgmat = ants.images_to_matrix(imglist, mask=mask2)

print(imgmat.sum()) # equals 0, but shouldnt I guess?

And tracing it more specifically:

import ants
import numpy as np

img = ants.image_read(ants.get_data('r16'))

s = [65]*img.dimension
mask = ants.from_numpy(np.random.randn(*s))
mask = mask > mask.mean()

img2 = ants.resample_image_to_target(img, mask, 2)

print(img2.sum()) # all zero
cookpa commented 4 months ago

Thanks! Can't fix now but can look at it later

ncullen93 commented 4 months ago

No prob.. think it has to do with spacing. I removed that listfunc thing though so maybe that will fix the error at least... it's like a function within a function trying to access the mask which isn't really in the scope of the inner function.

cookpa commented 4 months ago

One more thought before I have to run, should this

https://github.com/ANTsX/ANTsPy/blob/cd2a4acafc9fa38ea435de8fb9160522f024f242/ants/registration/apply_transforms.py#L121

be a clone of fixed, rather than moving? Since the output will be written to an image of dimension fixed. Not sure if it matters, seems like it would have been noticed by now if so.

Just worried about these intermittent access violations in light of issues like #590

cookpa commented 4 months ago

Answering my own question, it's fine to have

warpedmovout = moving.clone(output_pixel_type)

because the pointer will be reassigned to point to the output image created inside the C++ code, and it doesn't matter if the output image has more or less pixels.

cookpa commented 4 months ago

I also noticed resample_image_to_target (and ants.apply_transforms) will reset the output data type to the target type. So

img = ants.image_read(ants.get_data('r16')) mask = ants.image_clone( img > img.mean() ) mask ANTsImage Pixel Type : unsigned char (uint8) Components : 1 Dimensions : (256, 256) Spacing : (1.0, 1.0) Origin : (0.0, 0.0) Direction : [1. 0. 0. 1.]

x = ants.resample_image_to_target(img, mask, 'linear') x ANTsImage Pixel Type : unsigned char (uint8) Components : 1 Dimensions : (256, 256) Spacing : (1.0, 1.0) Origin : (0.0, 0.0) Direction : [1. 0. 0. 1.]

I think this shouldn't cause the memory error because the values should get cast to the matrix type, but it is different to how the CLI behaves. Needs documenting if nothing else

cookpa commented 4 months ago

Fixed by #642

cookpa commented 4 months ago

I looked again at the code vs the main apply_transforms, and one thing resample doesn't do is clone the input images to float, so I changed this in #647