JuliaImages / ImageDistances.jl

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

README `evaluate` example not working #15

Closed xiaodaigh closed 5 years ago

xiaodaigh commented 5 years ago
using Images, ImageDistances

img1 = rand(640,480); 
img2 = rand(640,480);

d = ModifiedHausdorff()

# distance between two images
evaluate(d, imgA, imgB) # 

hausdorff(img1, img2) # works but gives 0
juliohm commented 5 years ago

@johnnychen94 could you please take a look at this? I know you've been reviewing the code recently... I am guessing that the error has something to do with the fact that we are not importing Distances anymore.

Also, could you please submit a PR adding a comment that Hausdorff distances are only defined for black and white images?

@xiaodaigh could you please confirm that hausdorff(rand(0:1, 100,100), rand(0:1, 100, 100) returns something different than zero?

xiaodaigh commented 5 years ago

I do get 0 after running hausdorff(rand(0:1, 100,100), rand(0:1, 100, 100)). In fact I get

julia> hausdorff(rand(0:1, 100,100), rand(0:1, 100, 100))
┌ Warning: implicit `dims=2` argument now has to be passed explicitly to specify that distances between columns should be computed
│   caller = ip:0x0
└ @ Core :-1
0.0
johnnychen94 commented 5 years ago

Sure I’ll check it after I finished the redesign part. There’s some issue in my PR to Distances.jl, I need to think it carefully.

Johnny

johnnychen94 commented 5 years ago

could you please submit a PR adding a comment that Hausdorff distances are only defined for black and white images?

@juliohm you mean it's AbstractArray{Bool} type?

juliohm commented 5 years ago

Maybe we should consider restricting it to AbstractArray{Bool}, I do not remember the types I used in the implementation.

Typing very quickly without actually looking into the code. Very busy week around here...

Em seg, 29 de abr de 2019 às 12:14, Johnny Chen notifications@github.com escreveu:

could you please submit a PR adding a comment that Hausdorff distances are only defined for black and white images?

@juliohm https://github.com/juliohm you mean it's AbstractArray{Bool} type?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaImages/ImageDistances.jl/issues/15#issuecomment-487620646, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZQW3MBOZWQ3NPMIUFYNDDPS4GF5ANCNFSM4HI7WDRQ .

johnnychen94 commented 5 years ago

I am guessing that the error has something to do with the fact that we are not importing Distances anymore.

I've checked the ImageDistances@0.1.0, which is prior to my fix PR "remove unnecessary dependency on Distances" #8. Got the same 0.0 result. Perhaps we need to revisit the implementations.

juliohm commented 5 years ago

The warning in the example above in one of the previous comments is suspicious. Probably the bug is due to the update in distances.jl where it is now important to specify dim=2 or something... i can try take a look tomorrow if you can't find the bug earlier. Are the tests passing?

On Tue, Apr 30, 2019, 06:36 Johnny Chen notifications@github.com wrote:

I am guessing that the error has something to do with the fact that we are not importing Distances anymore.

I've checked the ImageDistances@0.1.0, which is prior to my fix PR "remove unnecessary dependency on Distances" #8 https://github.com/JuliaImages/ImageDistances.jl/pull/8. Got the same 0.0 result. Perhaps we need to revisit the implementations.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaImages/ImageDistances.jl/issues/15#issuecomment-487887936, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZQW3JYMTHREGRMKNIZBKDPTAHINANCNFSM4HI7WDRQ .

juliohm commented 5 years ago

@xiaodaigh sorry for the problem you had. I had incorrectly assigned the wrong reduction operation in the Hausdorff definition. When I wrote this package I only used ModifiedHausdorff() because it was shown to be superior in speed and object matching (see Dubuisson et al). I have now fixed the definition of Hausdorff in the code.

We should tag a patch release for this bug @johnnychen94 :+1: