Resolves problem of incorrect handing of NaN values #19
Conversation
| out, offsets = detect_subpixel_edges(img) | ||
| @test_reference "References/circle_edge.png" Gray.(detect_edges(img)) by=edge_detection_equality() |
There was a problem hiding this comment.
do we need to remove circle_edge.png?
FYI, one way to quickly rebuild all the references is to remove the whole References folder.
There was a problem hiding this comment.
For this particular test we unfortunately need to remove it. The issue is that by using the KernelFactors.gaussian() instead of KernelFactors.IIRGaussian() the resulting image is different in a subtle way. The image has fewer unique intensity values. This subtle changes messes up the implicit threshold that is associated with the "Default Values" of low = Percentile(20) and high = Percentile(80). Whereas these percentiles gave the correct result and were good default values for this particular edge image when we used the IIRGaussian filter, they are a poor choice for this particular edge image when using the KernelFactors.gaussian() filters. A suitable threshold for this image would now be low = Percentile(75) and high = Percentile(80). The trouble is that this test is specifically meant to execute the API for the default values. Hence, I decided to go with a different image altogether because it doesn't make sense to change the default values to low = Percentile(75) and high = Percentile(80) in the general case.
There was a problem hiding this comment.
Oh sorry, I didn't notice this circle_edge.png is used in L108. I was thinking of whether this file can be removed.
BTW, it seems that you've accidentally committed two cameraman_edge.png files.
There was a problem hiding this comment.
I might just add that this was a very annoying and perplexing discovery. :)
There was a problem hiding this comment.
and yet some bad luck in the nightly version :(
9dd750e to
82fe716
Compare
The Gaussian filtering step associated with the Canny edge detector used `KernelFactors.IIRGaussian((σ,σ))` which does not leave the NaN values untouched but returns what seem to be spurious values in the NaN region. Consequently, when one computes the gradient one obtains a gradient response in those regions, which in turns cause canny to detect edges there as well. I address this issue by swapping to a `KernelFactors.gaussian((σ,σ))` implementation. This should, in principle, yield a more accurate results. In practice, it means that the choice of `σ` had greater influence on the detected edges (the algorithm becomes more sensitive to the choice of that parameter). This meant that I had to rejigger some of the reference and test parameters in the test suite.
82fe716 to
6928b5e
Compare
The Gaussian filtering step associated with the Canny edge detector used
KernelFactors.IIRGaussian((σ,σ))which does not leave the NaN values untouched but returns what seem to be spurious values in the NaN region. Consequently, when one computes the gradient one obtains a gradient response in those regions, which in turns cause canny to detect edges there as well. I address this issue by swapping to aKernelFactors.gaussian((σ,σ))implementation. This should, in principle, yield a more accurate result. In practice, it means that the choice ofσhad greater influence on the detected edges (the algorithm becomes more sensitive to the choice of that parameter). This meant that I had to rejigger some of the reference and test parameters in the test suite.