search

JuliaImages / ImageDistances.jl

Distances between N-dimensional images
https://github.com/JuliaImages/Images.jl
Other
15 stars 8 forks source link

Random CI failure on Hausdorff and NCC #44

Closed johnnychen94 closed 3 years ago

johnnychen94 commented 4 years ago
mateuszbaran commented 3 years ago

The Hausdorff failure is probably due to imgA or imgB being all false here: https://github.com/JuliaImages/ImageDistances.jl/blob/a62631004394216c6561ee6af680add92bb9a598/src/hausdorff.jl#L104 so they are not empty but should be special-cased as well. I think there should be !any instead of isempty. I can make a PR with a fix if you want.

johnnychen94 commented 3 years ago

@juliohm could you help confirm this?

juliohm commented 3 years ago

Thanks for pinging @johnnychen94 👍🏽 I think @mateuszbaran is completely right here. I think this isempty check is from the previous implementation where we actually constructed point clouds for the active pixels. It should be replaced by !any as suggested. Thank you for spotting this @mateuszbaran , let's see if the tests pass after the change.

mateuszbaran commented 3 years ago

The NCC problem appears because that variant of NCC is NaN when one of the images is uniform, and it may sometimes happen. In such case we compare in a test isaprox(NaN, NaN) which of course fails. This can be fixed either by relaxing the test (either isapprox or both are NaN) or making random images such that they are never uniform.

By the way, your definition of NCC seems more related to ZNCC? See here: https://www.ncbi.nlm.nih.gov/pmc/articles/PMC3596837/ . Implementation and the docstring are inconsistent about subtracting the mean.

johnnychen94 commented 3 years ago

Oh, I just missed the last comment!

By the way, your definition of NCC seems more related to ZNCC? See here: https://www.ncbi.nlm.nih.gov/pmc/articles/PMC3596837/ . Implementation and the docstring are inconsistent about subtracting the mean.

Yes, it turns out the definition of NCC is wrong; the original version comes from Images.jl https://github.com/JuliaImages/ImageDistances.jl/pull/35/files I just created a new issue for this.

The NCC problem appears because that variant of NCC is NaN when one of the images is uniform, and it may sometimes happen

I've fixed the NaN issue in #58 with a nearlysame helper:

nearlysame(x, y) = x ≈ y || (isnan(x) & isnan(y))
nearlysame(A::AbstractArray, B::AbstractArray) = all(map(nearlysame, A, B))