[DFT] Rearrange DFT compute tests so unimplemented always skips

[FMarno]

Description

Since #288, MKLGPU now fails the REAL_REAL tests because the descriptor commit throws (due to not supporting REAL_REAL for COMPLEX_STORAGE). I've rearranged the tests so they always skip when something is not implemented.

To reproduce run the tests with the MKLGPU backend. The complex-to-complex *real_real* test will fail.

Fixes # (GitHub issue)

Checklist

All Submissions

• Do all unit tests pass locally? Attach a log.
• Have you formatted the code using clang-format?

Bug fixes

• Have you added relevant regression tests?
• Have you included information on how to reproduce the issue (either in a
  GitHub issue or in this PR)?

[hjabird]

The code itself looks good, but I don't understand the need to move from return test_skipped to GTEST_SKIP.

The original use of test_skipped seems more consistent with the tests from the other backends (eg. here). Can you comment on why GTEST_SKIP is needed in something that seems otherwise functionally equivalent?

[FMarno]

mklgpu_run.log

[FMarno]

The code itself looks good, but I don't understand the need to move from return test_skipped to GTEST_SKIP.

The original use of test_skipped seems more consistent with the tests from the other backends (eg. here). Can you comment on why GTEST_SKIP is needed in something that seems otherwise functionally equivalent?

test_skipped and EXPECT_TRUEORSKIP are oneMKL specific and will internally call GTEST_SKIP. Since the catch occurs at the top level of the test I would have to do EXPECT_TRUEORSKIP(test_skipped) but I just went straight to the GTEST_SKIP.

[FMarno]

mklgpu_run.log
Updated log

[anantsrivastava30]

Do you also think you can move

    if constexpr (domain == oneapi::mkl::dft::domain::REAL) {
        std::cout << "skipping real split tests as they are not supported" << std::endl;
        return test_skipped;
    }

this bit from the real_real test and skip the REAL domain from the compute_tests.cpp as a part of this PR

[anantsrivastava30]

Please attach the CPU logs as well

[FMarno]

Do you also think you can move

    if constexpr (domain == oneapi::mkl::dft::domain::REAL) {
        std::cout << "skipping real split tests as they are not supported" << std::endl;
        return test_skipped;
    }

this bit from the real_real test and skip the REAL domain from the compute_tests.cpp as a part of this PR

I would rather save that for a separate PR, I need to convince myself that its impossible for that to be implemented. Also I see it as a separate issue.
I did have the same thought though so most of the work is done here https://github.com/oneapi-src/oneMKL/compare/develop...FMarno:oneMKL:no_real_real_real2complex?expand=1

[FMarno]

Please attach the CPU logs as well

mklcpu_run.log
mklcpu log

[anantsrivastava30]

this time around the logs have this :

977/977 Test #977: dft/EXAMPLE/CT/complex_fwd_buffer_mklcpu .........................................................................................................................................   Passed    0.30 sec

100% tests passed, 0 tests failed out of 977

Total Test time (real) = 260.39 sec

The following tests did not run:
	140 - DFT/RT/ComputeTestSuite/ComputeTests_real_real_in_place.REAL_SINGLE_in_place_real_real_buffer/sizes_8_batches_1_11th_Gen_Intel_R__Core_TM__i9_11900K___3_50GHz (Skipped)
...
...

but previously the behavior was to show these as passed but in the full logs (sample is from running ctest as well

1937 968/968 Test #1943: DFT/CT/DescriptorCommitTestSuite/DescriptorCommitTests.DescriptorCommitTestsComplexDouble/Intel_R__Core_TM__i7_6770HQ_CPU___2_60GHz .........................................   Passed    0.81 sec
1938
1939 100% tests passed, 0 tests failed out of 968
<EOF>

old LastTest.log:

 3660 [ RUN      ] ComputeTestSuite/ComputeTests_real_real_in_place.REAL_SINGLE_in_place_real_real_buffer/sizes_8_batches_1_Intel_R__Core_TM__i7_6770HQ_CPU___2_60GHz
 3661 skipping real split tests as they are not supported
 3662 [  SKIPPED ] ComputeTestSuite/ComputeTests_real_real_in_place.REAL_SINGLE_in_place_real_real_buffer/sizes_8_batches_1_Intel_R__Core_TM__i7_6770HQ_CPU___2_60GHz (78 ms)
 3663 [----------] 1 test from ComputeTestSuite/ComputeTests_real_real_in_place (78 ms total)
 3664
 3665 [----------] Global test environment tear-down
 3666 [==========] 1 test from 1 test suite ran. (78 ms total)
 3667 [  PASSED  ] 0 tests.

what attributes this change in behavior ?

[FMarno]

The test log is from a run of ctest. the example you gave will have come from directly running test_main_dft_ct or test_main_dft_rt

[FMarno]

@anantsrivastava30 are you satisfied with this answer? Is there anything you would like clarified/changed for this to be approved?

[anantsrivastava30]

@anantsrivastava30 are you satisfied with this answer? Is there anything you would like clarified/changed for this to be approved?

I updated my initial question to indicate that I am running tests using ctest as well, and I see this huge tail at the end of every ctest run telling me that : these 100 odd tests did not run, I would have preferred that information to stay in the logs.

I however do not see that as a huge concern and every thing else looks good.

[anantsrivastava30]

Approved

[FMarno]

@anantsrivastava30 Thanks for clarifying. Unfortunately I don't see the same behaviour on develop (sha 4c7e7eb). I always see 977 tests running with the real to complex "real_real" tests being skipped.