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

resample_image_to_target / images_to_matrix #641

Closed
cookpa opened this issue May 18, 2024 · 9 comments
Closed

resample_image_to_target / images_to_matrix #641

cookpa opened this issue May 18, 2024 · 9 comments
Labels

Comments

@cookpa
Copy link
Member

cookpa commented May 18, 2024

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

x = ants.resample_image_to_target(x, mask, 2)

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

def resample_image_to_target(image, target, interp_type='linear', imagetype=0, verbose=False, **kwargs):

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

mytx = ['-t', 'identity']

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.

@cookpa cookpa added the bug label May 18, 2024
@ncullen93
Copy link
Member

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
Copy link
Member

ncullen93 commented May 18, 2024

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
Copy link
Member Author

cookpa commented May 18, 2024

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

@ncullen93
Copy link
Member

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
Copy link
Member Author

cookpa commented May 18, 2024

One more thought before I have to run, should this

warpedmovout = moving.clone(output_pixel_type)

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
Copy link
Member Author

cookpa commented May 18, 2024

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
Copy link
Member Author

cookpa commented May 18, 2024

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
Copy link
Member Author

cookpa commented May 20, 2024

Fixed by #642

@cookpa cookpa closed this as completed May 20, 2024
@cookpa
Copy link
Member Author

cookpa commented May 21, 2024

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

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

No branches or pull requests

2 participants