syclacademy icon indicating copy to clipboard operation
syclacademy copied to clipboard

Problem with exercise 16

Open lrpablo opened this issue 1 year ago • 1 comments

The solution.cpp uses items in the parallel_for but the README.md uses ids. I changed the README.md and added the code to the solution.cpp

lrpablo avatar Jan 09 '24 13:01 lrpablo

Hi,

To achieve a better match with as few changes to the code as possible, I think this can be a good solution:

cgh.parallel_for<image_convolution>(
    ndRange, [=](sycl::id<2> idx) {

        auto rowMajorLinearId = idx[1] * inputImgWidth + idx[0];
        auto columnMajorLinearId = idx[0] * inputImgWidth + idx[1];

        auto globalId(rowMajorLinearId % inputImgWidth, rowMajorLinearId / inputImgWidth);

what do you think ?

________________________________
From: Duncan McBain ***@***.***>
Sent: 31 January 2024 18:46
To: codeplaysoftware/syclacademy ***@***.***>
Cc: Pablo ***@***.***>; Author ***@***.***>
Subject: Re: [codeplaysoftware/syclacademy] Problem with exercise 16 (PR #326)


@DuncanMcBain commented on this pull request.

________________________________

In Code_Exercises/Exercise_16_Coalesced_Global_Memory/README.md<https://github.com/codeplaysoftware/syclacademy/pull/326#discussion_r1473302600>:

> @@ -17,8 +17,8 @@ global memory access patterns in your kernel are coalesced.
 Consider two alternative ways to linearize the global id:

-auto rowMajorLinearId = (idx[1] * width) + idx[0]; // row-major -auto columnMajorLinearId = (idx[0] * height) + idx[1]; // column-major +auto rowMajorLinearId = sycl::id(globalId[0], globalId[1]);

I don't think this is the original intent, here, since it talks about the linear id presumably the intent is to linearise it manually (to demonstrate the difference in striding). Can we change the other side instead to better match this?


In Code_Exercises/Exercise_16_Coalesced_Global_Memory/solution.cpphttps://github.com/codeplaysoftware/syclacademy/pull/326#discussion_r1473308010:

  •                auto rowMajorLinearId    = sycl::id(globalId[0], globalId[1]);
    
  •                auto columnMajorLinearId = sycl::id(globalId[1], globalId[0]);
    

Similarly these are 2d ids, not linear ones

— Reply to this email directly, view it on GitHubhttps://github.com/codeplaysoftware/syclacademy/pull/326#pullrequestreview-1854566344, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BE6TABJLX5HC53VYXKKNMSTYRKGQDAVCNFSM6AAAAABBTCWAX2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJUGU3DMMZUGQ. You are receiving this because you authored the thread.Message ID: @.***>

lrpablo avatar Feb 05 '24 18:02 lrpablo