-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix sub-group 'of' tests #807
Conversation
As discovered/discussed in KhronosGroup#795, the `predicate_function_of_sub_group` test was using the global linear ID to perform sub-group 'of' checks. The checks were only being run on the first sub-group (with ID 0). This was implicitly relying on the first sub-group containing the work-items with IDs from 0 up to the sub-group size, as the checks included conditions like exactly one ID being 1. This is not correct in general, as the mapping between work-items and sub-group items is not defined. In addition, this commit also enhances the sub-group tests to check that *all* sub-groups in the work-group return the correct result. Before we were either checking only the *first* sub-group (`predicate_function_of_sub_group`) or checking all sub-groups but overwriting the results each time, so in effect only checking the *last* sub-group. Now the results are initialized to 'true' and each sub-group is run and its results are merged with all of the other results.
That sounds good. Probably apply some of #804 (comment) |
I've added some extra comments now. I'm not sure exactly what suggestions you were making, so hopefully this is what you had in mind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite clearer! \o/
Co-authored-by: Ronan Keryell <[email protected]>
I don't suppose someone would be able/willing to squash this branch when merging? It's become a little messy (at least to my mind). No worries either way, though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I prefer keeping the history even it is messy, otherwise this is no longer history. :-)
As discovered/discussed in
#795, the
predicate_function_of_sub_group
test was using the global linear ID to perform sub-group 'of' checks.The checks were only being run on the first sub-group (with ID 0). This was implicitly relying on the first sub-group containing the work-items with IDs from 0 up to the sub-group size, as the checks included conditions like exactly one ID being 1. This is not correct in general, as the mapping between work-items and sub-group items is not defined.
In addition, this commit also enhances the sub-group tests to check that all sub-groups in the work-group return the correct result. Before we were either checking only the first sub-group
(
predicate_function_of_sub_group
) or checking all sub-groups but overwriting the results each time, so in effect only checking the last sub-group.Now the results are initialized to 'true' and each sub-group is run and its results are merged with all of the other results.