[clang-format] Reformat due to transition from v9.0.0 to v19.1.0 and update of style configuration

[dnhsieh-intel]

Description

This PR is a preparation step for issue #580. Since we will move from clang-format v9.0.0 to v19.1.0, this PR reformats the code according to v19.1.0. This PR is separated from the CI enabling (PR #595) in order to have a more informative git blame.

There are some changes I want to discuss.

• src/lapack/backends/cusolver/cusolver_lapack.cpp
  The following changes are suggested by clang-format:

  -    queue.submit([&](sycl::handler &cgh) {
  -        onemkl_cusolver_host_task(cgh, queue, [=](CusolverScopedContextHandler &sc) {
  -            auto handle = sc.get_handle(queue);
  -            cusolverStatus_t err;
  -            CUSOLVER_ERROR_FUNC_T_SYNC(func_name, func, err, handle, m, n, scratch_size);
  -        });
  -    }).wait();
  +    queue
  +        .submit([&](sycl::handler &cgh) {
  +            onemkl_cusolver_host_task(cgh, queue, [=](CusolverScopedContextHandler &sc) {
  +                auto handle = sc.get_handle(queue);
  +                cusolverStatus_t err;
  +                CUSOLVER_ERROR_FUNC_T_SYNC(func_name, func, err, handle, m, n, scratch_size);
  +            });
  +        })
  +        .wait();

  Because it looks too funny, I added // clang-format off/on for all the similar suggestions in this file.
• tests/unit_tests/dft/include/{compute_out_of_place,compute_inplace}.hpp
  Although I don't like the following formatting personally, it is probably still fine. I can turn the formatting off if there are objections.

  -                EXPECT_TRUE(check_equal_strided<domain == oneapi::mkl::dft::domain::REAL>(
  -                    bwd_ptr + backward_distance * i, out_host_ref.data() + ref_distance * i, sizes,
  -                    strides_bwd_cpy, abs_error_margin, rel_error_margin, std::cout));
  +                EXPECT_TRUE(check_equal_strided < domain ==
  +                            oneapi::mkl::dft::domain::REAL >
  +                                (bwd_ptr + backward_distance * i,
  +                                 out_host_ref.data() + ref_distance * i, sizes, strides_bwd_cpy,
  +                                 abs_error_margin, rel_error_margin, std::cout));

Checklist

All Submissions

• Do all unit tests pass locally? Only code reformatting. The CPU tests in CI passed.
• Have you formatted the code using clang-format?

[Rbiessy]

The diff looks good to me overall.
Regarding the changes in src/lapack/backends/cusolver/cusolver_lapack.cpp, could we try to change the clang-format configuration file to get a good enough formatting without // clang-format off/on?
Looking at the repository's _clang-format and the documentation there are a bunch of deprecated configurations that we should update:

• Standard: Cpp11 -> Standard: c++17 (or Standard: Latest)
• AllowAllConstructorInitializersOnNextLine and ConstructorInitializerAllOnOneLineOrOnePerLine can be replaced with PackConstructorInitializers: Never
• AlwaysBreakAfterDefinitionReturnType can be removed
• AlwaysBreakAfterReturnType -> BreakAfterReturnType: Automatic
• AlwaysBreakTemplateDeclarations -> BreakTemplateDeclarations: Yes
• KeepEmptyLinesAtTheStartOfBlocks ->

  KeepEmptyLines:
    AtEndOfFile: true
    AtStartOfBlock: false
    AtStartOfFile: false
  

• SpaceInEmptyParentheses -> SpacesInParens: Never

Maybe this will be enough to fix the issue. If not we could maybe add the configuration PenaltyIndentedWhitespace: 500 for instance.

[andrewtbarker]

I agree with @Rbiessy that we should try to improve the configuration file rather than using // clang-format off. This PR is a good opportunity to improve the config file.

[dnhsieh-intel]

It looks like this is just how clang-format handles method chaining:

    queue
        .submit([&](sycl::handler& cgh) {
            // body
            });
        })
        .wait();

I didn't find an option for this unfortunately. One workaround is to write queue.submit() and queue.wait() separately, but this is against the purpose of using automatic formatting. I think we just let clang-format do its job...

The style configuration file is updated in the commit ff81d95. Most changes and clean-up are straightforward. However, one significant change is DerivePointerAlignment: false. The description of this option is

If true, analyze the formatted file for the most common alignment of & and *. Pointer and reference alignment styles are going to be updated according to the preferences found in the file. PointerAlignment is then used only as fallback.

Because the alignment style is according to the preferences found in the file, the current code base has both left and right alignments, for example,
https://github.com/oneapi-src/oneMKL/blob/6923d402d5bccba9ae1966062bc5a277fc74776c/examples/blas/run_time_dispatching/level3/gemm_usm.cpp#L63
https://github.com/oneapi-src/oneMKL/blob/6923d402d5bccba9ae1966062bc5a277fc74776c/examples/blas/compile_time_dispatching/level3/gemm_usm_mklcpu_cublas.cpp#L64
I am open to either left or right alignment, but we must choose one in my opinion.

[Rbiessy]

The changes look good to me, thanks.
I much prefer the left alignment of pointers and reference!
I don't mind too much about the queue.[...].wait() pattern. As you said we can split these lines if needed later.

[Rbiessy]

I didn't mean to approve on behalf of the other teams, I don't know of way to change this in GitHub so I have just requested for their approval again!

[andrewtbarker]

Thanks for doing this.

[sknepper]

Thank you for all your work here, @dnhsieh-intel !
This is wonderful - looking forward to the upcoming CI enablement as well!

[sknepper]

I have a slight preference for the previous way this was formatted (each clause on its own line), but I don't think we have too many places where this happens, and it's still understandable.

[dnhsieh-intel]

I think (almost) each clause on its own line was introduced in PR #571, and we haven't fixed its formatting. Actually clang-format-9 and the old style file would give the same result.

[dnhsieh-intel]

@oneapi-src/onemkl-rng-write, @oneapi-src/onemkl-sparse-write, @oneapi-src/onemkl-dft-write I plan to merge the PR on 10/22 EOD Pacific time if there are no objections.

[anantsrivastava30]

Looks good to me, this is the only point that I prefer the old syntax as it's slightly more readable, but as these are unit test its fine.
Thank you for this effort.