Docs clarification for functions downstream of imgradients: g1, g2 vs gy, gx

[stillyslalom]

Many of the edge-detection functions require the gradients of an image, e.g. orientation(grad_x, grad_y). However, imgradients now delivers gradients along each axis of an image (g1, g2, ...), but it seems that the idea hasn't propagated through the entirety of the codebase:

julia> grad_y, grad_x, mag, orient = imedge(img, KernelFactors.bickley)

julia> orientation(grad_y, grad_x) == orient # true

help?> orientation

  orientation(grad_x, grad_y) -> orient

  Calculate the orientation angle of the strongest edge from gradient images 
given by grad_x and grad_y. Equivalent to atan(grad_x, grad_y). When both 
grad_x and grad_y are effectively zero, the corresponding angle is set to zero.

[timholy]

Oh dear! Thanks for noticing this. It will take some thought about how to fix this; switching argument order is just about impossible to deprecate.

[zygmuntszpak]

As part of our move to a consistent API I was contemplating the creation of ImageEdgeDetection.jl which functions such as detect_edges(img, Canny(args...). Perhaps we can introduce functions with slight different names such as determine_orientation which allows one to specify the co-ordinate system convention. Users can then migrate their code to the new API and the old functions can call the new one with their old co-ordinate system conventions.

[stillyslalom]

Another option: returning a NamedTuple grads=(gy, gx) from imgradients, and adding new single-argument methods for all the downstream edge-detection functions:

gy, gx = grads = imgradients(img, KernelFactors.bickley)
magnitude(grads)
phase(grads)

These methods could also take information from image traits, e.g. spacedirections. One of my longstanding gripes with image processing is the translation from physical coordinates (x, y, z) to image coordinates (-y, x, z), which makes the trigonometry of these sorts of functions harder to reason about.

[stillyslalom]

It's worth taking a step back when thinking about these functions (and the imgradients ecosystem, ref. JuliaImages/ImageFiltering.jl#142) and how they apply to arrays with non-(image)-standard spatial axes. I use imgradients to process simulation data from rectilinear & curvilinear grids, but I've run into a number of challenges downstream of the disconnect between image & physical coordinates, and it'd be great to just provide my physical axes as a trait and be assured that the right-hand rule will hold true.

The most consistent language for this domain is that of tensors, and the 'theory manual' portion of the docs (thanks Zygmunt!) already hews to that. It'd be wonderful to approach ArrayFiltering from that direction to provide a more complete & consistent treatment of spatial derivatives/vector calculus.

[timholy]

This name and way of thinking is basically descended from tradition established in previous programming languages. I think we're already a bit ahead of the curve given that, e.g., sobel actually takes a gradient rather than 8x a gradient like it seems to in most other languages.

To do better than we are now, though, I think one would need to figure out some way to indicate which coordinates one is taking the gradient with respect to. Any ideas for notation? And how that would couple to array axis notations a la AxisArrays or similar?

[stillyslalom]

On further thought, I'm not sure that a massive algorithmic overhaul is required, but the documentation could be a bit clearer on the subject of axes/coordinates. For reference, compare the documentation of Gdir from Matlab's [Gmag, Gdir] = imgradient(img):

Gdir contains angles in degrees within the range [-180, 180] measured
counterclockwise from the positive x-axis. (The x-axis points in the 
direction of increasing column subscripts.) 

to Images.jl

  orientation(grad_x, grad_y) -> orient

  Calculate the orientation angle of the strongest edge from gradient images 
given by grad_x and grad_y. Equivalent to atan(grad_x, grad_y). When both 
grad_x and grad_y are effectively zero, the corresponding angle is set to zero.

The missing information: orientation relative to what? with an angle of rotation positive about which axis? The coordinate system & sign conventions for all these functions (imedge, imgradients, phase, orientation, ...) need to be clearly outlined somewhere in the documentation, but I don't think it's necessary to include it in each docstring, as long as the definition is easily accessible. That might require making a new domain-specific page & splitting the relevant information out of the function reference, which is currently in a state of uncertain balance between completeness & discoverability.

[stillyslalom]

More directly, we need to decide what orientation means. My PR #868 to patch this issue is failing because the preexisting test for imedge disagrees with the docstring for orientation:

img = zeros(2, 5)
img[:, 3] .= 1
grad_y, grad_x, mag, orient = imedge(img)
target_orient = zeros(2, 5); target_orient[:, 4] .= pi
@test orient ≈ target_orient            # true
@test atan.(grad_x, grad_y) ≈ orient    # false

julia> orient
2×5 Array{Float64,2}:
 0.0  0.0  0.0  3.14159  0.0
 0.0  0.0  0.0  3.14159  0.0

julia> atan.(grad_x, grad_y)
2×5 Array{Float64,2}:
 0.0  1.5708  0.0  -1.5708  0.0
 0.0  1.5708  0.0  -1.5708  0.0

help?> orientation
orientation(grad_x, grad_y) -> orient

  Calculate the orientation angle of the strongest edge from gradient images 
given by grad_x and grad_y. Equivalent to atan(grad_x, grad_y).

The test in question was written around the time when imgradients switched from returning grad_x, grad_y to grad_1, grad_2, ..., which may have caused the confusion. All the edge-finding functions (& user code) downstream of these conflicting definitions will need to be reconciled.

[timholy]

Thanks for the detective work @stillyslalom! I'm a bit too swamped to tackle this now, but I've added a 1.0 milestone so this doesn't get forgotten. (Hopefully we will get to it sooner than that.)

[johnnychen94]

I think this one is fixed by JuliaImages/ImageEdgeDetection.jl#10