oneDPL icon indicating copy to clipboard operation
oneDPL copied to clipboard

[onedpl][ranges][test] Added std ranges to oneDPL Tested API

Open MikeDvorskiy opened this issue 9 months ago • 4 comments

[onedpl][ranges][test] Added std ranges to oneDPL Tested API (for parallel oneDPL algorithms):

C++ standard sized RA views (factories and adaptors), released with C++20:

  1. views::all
  2. subrange
  3. single_view
  4. iota_view
  5. take_view
  6. drop_view
  7. reverse_view
  8. transform_view

MikeDvorskiy avatar May 08 '24 13:05 MikeDvorskiy

For other tested APIs we preferred to adjust libc++ tests, Have you looked at https://github.com/llvm/llvm-project/tree/main/libcxx/test/std/ranges? Does it make sense to use these for testing, and if not - why?

No, I have not looked at ".../libcxx/test/std/ranges" before... I've done it right now.... The first concern, that I have - a std test contains many RT assserts in a test body... It doesnt work in a kernel, as far as I understand. As variant we can "invent" some macro which "converts" each assert condition to "b_res &&= condition" and write RT assert after kernel on the host side with global variable "b_res" check. The second concern - there are many files on each std range. Each file checks just one part of functionality of the range. As far as we have some restrictions on std views/ranges (like sized RA views), probably we we need just the test files which test juts "sized RA views/ranges"

MikeDvorskiy avatar May 13 '24 09:05 MikeDvorskiy

RT asserts Update: it seems that works, it leads to std::abort call... (https://intel.github.io/llvm-docs/design/Assert.html)

MikeDvorskiy avatar May 13 '24 10:05 MikeDvorskiy

The second concern - there are many files on each std range. Each file checks just one part of functionality of the range.

as an example, I have adapted a test for "all_view" (as is with minimal changes). https://github.com/oneapi-src/oneDPL/pull/1571/files#diff-e827a6e11d65e046ef5e9e92d7c4302e71a32b3ac4557dd16f8a423762221701

Ok, for "all_view" there is one (two but we can take juts one) test file (from LLVM). BUT, for the others views LLVM has about 10 test files for each testing view! Each file checks juts one method. I am not sure that we want to have so bulk tests for our needs in oneDPL. In my opinion, checking the base functionality in 1-3 lines (see below) in one lambda is enough for oneDPL.

auto test = [](){
        auto res = std::ranges::views::iota(0, 4);
        return res.size() == 4 && res[0] == 0 && res[1] == 1 && res[2] == 2 && res[3] == 3 
                  && *(res.begin() + 2) == 2 && (res.end() - res.begin()) == 4;
    };

MikeDvorskiy avatar May 16 '24 09:05 MikeDvorskiy

The second concern - there are many files on each std range. Each file checks just one part of functionality of the range.

as an example, I have adapted a test for "all_view" (as is with minimal changes). https://github.com/oneapi-src/oneDPL/pull/1571/files#diff-e827a6e11d65e046ef5e9e92d7c4302e71a32b3ac4557dd16f8a423762221701

Ok, for "all_view" there is one (two but we can take juts one) test file (from LLVM). BUT, for the others views LLVM has about 10 test files for each testing view! Each file checks juts one method. I am not sure that we want to have so bulk tests for our needs in oneDPL. In my opinion, checking the base functionality in 1-3 lines (see below) in one lambda is enough for oneDPL.

auto test = [](){
        auto res = std::ranges::views::iota(0, 4);
        return res.size() == 4 && res[0] == 0 && res[1] == 1 && res[2] == 2 && res[3] == 3 
                  && *(res.begin() + 2) == 2 && (res.end() - res.begin()) == 4;
    };

From point of view of other our test implementations, it's good way to have one test for one function. We always able to se exactly what was broken.

SergeyKopienko avatar May 16 '24 09:05 SergeyKopienko