maikonadams / opencl-book-samples

Automatically exported from code.google.com/p/opencl-book-samples
0 stars 0 forks source link

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

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
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 wel...@gmail.com on 4 Aug 2011 at 2:34

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by dginsb...@upsamplesoftware.com on 6 Aug 2011 at 12:54

GoogleCodeExporter commented 8 years ago
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 emil.sty...@gmail.com on 11 Aug 2011 at 9:21

GoogleCodeExporter commented 8 years ago
Correct.

Original comment by wel...@gmail.com on 11 Aug 2011 at 10:38

GoogleCodeExporter commented 8 years ago
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 teneight...@gmail.com on 23 Sep 2011 at 4:48

GoogleCodeExporter commented 8 years ago
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 angelo.s...@gmail.com on 18 Nov 2011 at 11:38

GoogleCodeExporter commented 8 years ago
4th comment works good. Thank you for that code.

Original comment by sercan.y...@gmail.com on 14 Feb 2012 at 1:28