Fix height width order in dense_inference.py example

[kevin-keraudren]

The proper syntax is dcrf.DenseCRF2D(img.shape[1], img.shape[0], M).

This has to do with C-ordering versus Fortran ordering, also known as https://en.wikipedia.org/wiki/Row-major_order .

This bug can be highlighted with the following parameters:

d.addPairwiseGaussian(sxy=3, compat=0)
d.addPairwiseBilateral(sxy=3, srgb=5, rgbim=img, compat=50)

(img.shape[0], img.shape[1]) gives:

Note the straight lines characteristic of a stride misalignement.

On the other hand, (img.shape[1], img.shape[0]) gives:

This is confirmed by this other Python wrapper for densecrf:

See the order (D, W, H) at this line: https://github.com/mbickel/DenseInferenceWrapper/blob/master/denseinference/lib/libDenseCRF/densecrf.cpp#L139

while the original 2D code is (H, W).

[lucasb-eyer]

(Ping @swehrwein who fixed another ordering issue in the past.)

Your example images are very convincing. I am now somewhat confused, though. In the following, C-ordered means (H,W,C) and F-ordered (W,H,C) where C are channels.

1. The images in numpy loaded by cv2 are C-ordered.
2. According to the following comment[1] in densecrf's C++ example code, unary is C-ordered too.
3. The PPM that is loaded in the C++ example is also C-ordered.
4. We can see that this loop treats the bilateral image data in C-order too.
5. The link you give doesn't show any ordering of the data, just of the constructor's parameters. And at call-site it correctly passes img_col_ as W and img_row_ as H.
6. But in the same wrapper we can see here that data for the unary is re-packed from (H,W,Z,C) C-order into (Z,W,H,C) F-order. Despite this comment where it's used stating it being C-order.

So from all I see in densecrf's original C++ example and C++ code, everything seems C-ordered. The link to the other python wrapper you posted does re-package by transposing image dimensions, as does your suggestion.

Can you point me to where exactly in the library/example the need of transposing the spatial dimension is apparent?

I think this needs a cleaner fix than passing W where the library asks for H and vice-versa.

1:

// Specify the unary potential as an array of size W*H*(#classes)
// packing order: x0y0l0 x0y0l1 x0y0l2 .. x1y0l0 x1y0l1 ...
crf.setUnaryEnergy( unary );

[kevin-keraudren]

This is where we access the flattened numpy arrays: https://github.com/lucasb-eyer/pydensecrf/blob/master/densecrf/src/densecrf.cpp#L63-L67 https://github.com/lucasb-eyer/pydensecrf/blob/master/densecrf/src/densecrf.cpp#L72-L79

And regarding the pixel ordering of OpenCV images in Python, I refer you to this old thread: http://opencv-users.1802565.n2.nabble.com/OpenCV-y-x-points-for-numpy-x-y-points-td7054352.html#a7059295

Which can be summarised by:

height = img.shape[0] # y
width = img.shape[1] # x

Which is consistent with the C++ code:

DenseCRF2D::DenseCRF2D(int W, int H, int M) : DenseCRF(W*H,M), W_(W), H_(H) {
}

And the python code:

dcrf.DenseCRF2D(img.shape[1], img.shape[0], M)

[markusnagel]

I just found out (before seeing this PR) the same issue as Kevin. His argumentation is correct and after changing the dimensions the output of the c++ implementation and the python wrapper are the same (except small numeric difference).

The effect is also easy to see when removing the bilateral term in the example (which depends to the image) and give the location term a higher influence (compat=10). With dcrf.DenseCRF2D(img.shape[1], img.shape[0], M) we get which is inline with what you also get from the c++ implementation. However if you dcrf.DenseCRF2D(img.shape[0], img.shape[1], M) we get which is not in line with the c++ implementation and it also does visually not make much sense.

Please merge this PR, than not more people have to waste their time with a wrong example. Thanks

[lucasb-eyer]

Thanks for pinging, I lost track of this issue! Thanks to both of you for looking into it.