seung-lab / euclidean-distance-transform-3d

Euclidean distance & signed distance transform for multi-label 3D anisotropic images using marching parabolas.
GNU General Public License v3.0
234 stars 37 forks source link

[feature request] Check whether input is memory contiguous #10

Closed maweigert closed 5 years ago

maweigert commented 5 years ago

Hi,

Currently, the input data is assumed to be memory contiguous, yet this is not checked in the main function edt(data, ...), leading to wrong results for non contiguous (e.g. sliced, transpose) data without being noticed. I think having something like

if not data.flags['C_CONTIGUOUS'] or data.flags['F_CONTIGUOUS']:
   raise ValueError('data should be either C- or F-contiguous!')

at the beginning of edt would get rid of that source of error.

Thanks for the package!

william-silversmith commented 5 years ago

D'oh. I fixed that issue in a few of my other packages by doing np.copy(data, order=order) when the underlying memory is fragmented. If you don't have an objection, I'll use that technique here as it will make the operation of the library simpler at the cost of extra memory for the uninitiated.

Thanks for your feedback! Happy to see others are using this.

Will

On Sat, Jan 26, 2019, 9:23 AM Martin Weigert <notifications@github.com wrote:

Hi,

Currently, the input data is assumed to be memory contiguous, yet this is not checked in the main function edt(data, ...), leading to wrong results for non contiguous (e.g. sliced, transpose) data without being noticed. I think having something like

if not data.flags['C_CONTIGUOUS'] or data.flags['F_CONTIGUOUS']: raise ValueError('data should be either C- or F-contiguous!')

at the beginning of edt would get rid of that source of error.

Thanks for the package!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/seung-lab/euclidean-distance-transform-3d/issues/10, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZoSbBeOqQlw6txMnoOztkZAYMuaXfIks5vHGTxgaJpZM4aUMSS .

maweigert commented 5 years ago

Sure! :)

One could as well use np.ascontiguousarray(data), as it won't perform a copy when data is already contiguous.

Cheers, Martin

william-silversmith commented 5 years ago

This should be resolved in 1.2.3 via e0da4393748e74b28aa1841b29bcb9afc20f3240

Thanks for reporting!