oneDPL icon indicating copy to clipboard operation
oneDPL copied to clipboard

[test] Add SYCL canary test to identify bad environment / configuration

Open danhoeflinger opened this issue 1 year ago • 6 comments

This PR adds a SYCL canary test, which is meant to be divorced for oneDPL specifics, and launch a minimal sycl kernel.

This is meant to be used in CI runs, to identify for dpcpp backends when the SYCL environment / setup is invalid, and stop before continuing to main oneDPL tests.

This should provide more stability to aggregated test pass statistics for SYCL, especially on windows, more robust to environmental issues, and provide a tool to more concretely separate issues in oneDPL vs issues in setup.

danhoeflinger avatar Sep 06 '24 15:09 danhoeflinger

Do we want to name this something other than *.pass so it doesn't get included in our full test glob?

The internal CI specifically builds and runs this test separately, but there could be some advantage to have it included in the full build suite for runs outside the CI, so we can use it as a status there too.

danhoeflinger avatar Sep 06 '24 15:09 danhoeflinger

Do we want to name this something other than *.pass so it doesn't get included in our full test glob?

The internal CI specifically builds and runs this test separately, but there could be some advantage to have it included in the full build suite for runs outside the CI, so we can use it as a status there too.

I agree. Having it compiled/run a second time isn't an issue and the naming should be consistent with other tests we expect to pass.

timmiesmith avatar Sep 06 '24 15:09 timmiesmith

I'm going to hold on this PR until we have a system solid within the CI to handle it, in case other changes are found to be needed.

danhoeflinger avatar Sep 06 '24 15:09 danhoeflinger

@danhoeflinger, have you also considered replacing/complementing CMake checks of a compiler? It may be a more nature way to follow, compared to adding a new test along with other test. You can then add logic not to start testing if these checks do not pass, and this will not affect oneDPL pass rate.

You can try relying on https://cmake.org/cmake/help/v3.14/command/try_run.html, for example.

dmitriy-sobolev avatar Sep 06 '24 16:09 dmitriy-sobolev

@danhoeflinger, have you also considered replacing/complementing CMake checks of a compiler? It may be a more nature way to follow, compared to adding a new test along with other test. You can then add logic not to start testing if these checks do not pass, and this will not affect oneDPL pass rate.

You can try relying on https://cmake.org/cmake/help/v3.14/command/try_run.html, for example.

Yes, This was the first avenue I explored but decided against it for the following reasons:

  1. try_run / try_compile is not good with adding oneDPL as a link library and incorporating all the compiler flags etc. that come with that. I saw in the forums for CMake that because the config isn't complete at the time of the try_run command, it isn't able to use it as a link library and inherit the flags, etc. We also have specific environment variables we set for our tests. Therefore, it may be challenging and error prone to get it it in the same situation environment-wise with the rest of our tests, having to mirror every change with a matching one for the try_run command.
  2. try_run doesnt work so well with cross compilation

With this setup, we can very easily get the test in the same situation as our other tests from an environment + flag perspective. I agree, I would prefer not to add this test to the pass rate as a passing test for each system, but that's relatively minor in the scheme of things. We also could work out the command to exclude this test explicitly from the main run. We need to make sure that plays nice with the filter commands passed in via the CI run fields though. I will look into that.

danhoeflinger avatar Sep 06 '24 19:09 danhoeflinger

update on this... It seems that inclusion of a broken tbb install may be the bigger issue in our nightly windows CI testing. This may still be a good idea to pursue so I will keep it for now. Once the issue with TBB inclusion on windows in the CI testing is resolved I would like to revisit this.

danhoeflinger avatar Oct 21 '24 19:10 danhoeflinger