Errata: Wrong values in figure 3.2 (ch.3 pg.90)

[GoogleCodeExporter]

Figure 3.2 is meant to illustrate the process of convolution. However, wrong 
values in the 'result' matrix, may confuse the unsespecting reader.

These are the errors in the figure:
* The value in cell (1, 3) is 19, where it should be 27.
* The value in cell (6, 6) is 38, which is the correct result for the marked 
cells in the input matrix, however, these refer to cell (4, 5) of the output 
matrix, not cell (6, 6).

See attached file for the relevant figure.

Original issue reported on code.google.com by [email protected] on 4 Aug 2011 at 2:34

Attachments:

• err_ch03_pg90.png

[GoogleCodeExporter]

Original comment by [email protected] on 6 Aug 2011 at 12:54

[GoogleCodeExporter]

It's the code that is wrong - it looks like the output signal in the figure is 
copied from the output the code gives.

The error with the code is that the kernel expects a 2D work size, but the host 
code gives a 1D work size.  Also, the coordinates are reversed in the final 
printout of the result.

Original comment by [email protected] on 11 Aug 2011 at 9:21

[GoogleCodeExporter]

Correct.

Original comment by [email protected] on 11 Aug 2011 at 10:38

[GoogleCodeExporter]

I changed the code to this and it seems to do the job:

    const size_t globalWorkSize[2] = { outputSignalWidth, outputSignalHeight };
    const size_t localWorkSize[2]  = { 1, 1 };

    // Queue the kernel up for execution across the array
    errNum = clEnqueueNDRangeKernel(
        queue, 
        kernel, 
        2, 
        NULL,
                globalWorkSize, 
        localWorkSize,
                0, 
        NULL, 
        NULL);

[...]

    // Output the result buffer
    for (int x = 0; x < outputSignalHeight; x++)
    {
        for (int y = 0; y < outputSignalWidth; y++)
        {
            std::cout << outputSignal[x][y] << " ";
        }
        std::cout << std::endl;
    }

This is the first time I use OpenCL, so I would recommend someone look over it, 
if this would be the appropriate solution (and let me know how to do better, 
please ;))

Original comment by [email protected] on 23 Sep 2011 at 4:48

[GoogleCodeExporter]

There is a problem in the code from the very beginning.
Input, Mask and Output initializations should be like:

cl_uint inputSignal[inputSignalHeight][inputSignalWidth] =
{
    {3, 1, 1, 4, 8, 2, 1, 3},
    {4, 2, 1, 1, 2, 1, 2, 3},
    {4, 4, 4, 4, 3, 2, 2, 2},
    {9, 8, 3, 8, 9, 0, 0, 0},
    {9, 3, 3, 9, 0, 0, 0, 0},
    {0, 9, 0, 8, 0, 0, 0, 0},
    {3, 0, 8, 8, 9, 4, 4, 4},
    {5, 9, 8, 1, 8, 1, 1, 1}
};

cl_uint outputSignal[outputSignalHeight][outputSignalWidth];

cl_uint mask[maskHeight][maskWidth] =
{
    {1, 1, 1}, {1, 0, 1}, {1, 1, 1},
};

Which means that the matrix has to be defined using the [Height][Width] order 
and not the [Width][Height] order. 
Using this solution the output of the result buffer can be kept as it is BUT 
the x and y indexes need to be reversed:

std::cout << outputSignal[y][x] << " ";

instead of

std::cout << outputSignal[x][y] << " ";

I am not used to OpenCL so I don't know if the proposed code changing (giving 
the kernel a 2D buffer rather than a 1D one) is "correct", but it seems to be 
working fine.
I hope I have explained this clearly.

Original comment by [email protected] on 18 Nov 2011 at 11:38

[GoogleCodeExporter]

4th comment works good. Thank you for that code.

Original comment by [email protected] on 14 Feb 2012 at 1:28