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

ndimage routines behave badly when output has memory overlap with inputs #9553

Closed
skenop opened this issue Nov 29, 2018 · 6 comments · Fixed by #11986
Closed

ndimage routines behave badly when output has memory overlap with inputs #9553

skenop opened this issue Nov 29, 2018 · 6 comments · Fixed by #11986
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.ndimage
Milestone

Comments

@skenop
Copy link

skenop commented Nov 29, 2018

Maybe the problem is following
...
tmp = grey_erosion(tmp, size, footprint, structure, output, mode, cval, origin)
tmp and input is the same now
so
numpy.subtract(tmp, input, out=tmp)
gives zero

Thank you

@ilayn
Copy link
Member

ilayn commented Nov 29, 2018

@yuracpp Do you mind adding a reproducable code snippet that we can copy and paste to see the problem for ourselves?

It is not easy for us to understand anything from this.

@skenop
Copy link
Author

skenop commented Nov 29, 2018

import numpy as np
import scipy.ndimage as ndi

a = np.ones((3,3))
a[1,1] = 0
ndi.black_tophat(a, size=3)
array([[0., 0., 0.],
[0., 1., 0.],
[0., 0., 0.]])
ndi.black_tophat(a, size=3, output=a)
array([[0., 0., 0.],
[0., 0., 0.],
[0., 0., 0.]])

@akahard2dj
Copy link
Contributor

In this part, when output variable gets in grey_erosion(), input data is always changed.
But, problem was the case of input = output.
I have designed the solution by comparing input and output.

the results are always same.

    if numpy.array_equal(input, output):
        tmp = grey_erosion(tmp, size, footprint, structure, None, mode,
                          cval, origin)
    else:
        tmp = grey_erosion(tmp, size, footprint, structure, output, mode,
                           cval, origin)

issue reproduce code

import numpy as np
import scipy.ndimage as ndi

a = np.ones((3,3))
a[1,1] = 0
a=ndi.black_tophat(a, size=3)
print('result1')
print(a)

a = np.ones((3,3))
a[1,1] = 0
a=ndi.black_tophat(a, size=3, output=a)
print('result2')
print(a)

a = np.ones((3,3))
a[1,1] = 0
b=np.ones((3,3))
a=ndi.black_tophat(a, size=3, output=b)
print('result3')
print(a)
print(b)

results

result1
[[0. 0. 0.]
 [0. 1. 0.]
 [0. 0. 0.]]

result2
[[0. 0. 0.]
 [0. 1. 0.]
 [0. 0. 0.]]

result3
[[0. 0. 0.]
 [0. 1. 0.]
 [0. 0. 0.]]
[[0. 0. 0.]
 [0. 1. 0.]
 [0. 0. 0.]]

@skenop
Copy link
Author

skenop commented Dec 3, 2018

thanks, the output is correct with this fix.
BTW white_tophat has symmetric issue

a = np.zeros((3,3))
a[1,1] = 1
print(ndi.white_tophat(a, size=3))
[[0. 0. 0.]
[0. 1. 0.]
[0. 0. 0.]]
print(ndi.white_tophat(a, size=3, output=a))
[[0. 0. 0.]
[0. 0. 0.]
[0. 0. 0.]]

@akahard2dj
Copy link
Contributor

Yes, white_tophat() function has same problem.
I will fix them and PR.

@pv
Copy link
Member

pv commented Aug 7, 2019

The problem is that these routines do not check if the output array overlaps in memory with the inputs.

As mentioned in #9578, same issue is in rank_filter and likely many other functions in ndimage.

Since the ndimage code is not written to enable inplace operations, the fix has to be to make copies along the lines of if np.may_share_memory(output, input): input = input.copy() --- or some equivalent --- in these functions. Possibly, it is simpler to do this in the C routines, in case it could be done in a single place in them.

@pv pv changed the title black_tophat returns zeros if black_tophat(input, ..., output=input) ndimage routines behave badly when output has memory overlap with inputs Aug 7, 2019
grlee77 added a commit to grlee77/dipy that referenced this issue May 1, 2020
input/output arrays should not overlap in calls to scipy.ndimage.median
see scipy/scipy#9553
will be fixed upstream in scipy/scipy#11986, but the solution here will
avoid repeated temporary array creation in each iteration.
@rgommers rgommers added this to the 1.5.0 milestone May 4, 2020
@rgommers rgommers added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.ndimage
Projects
None yet
6 participants