[SYCL][COMPLEX] Split complex header file and tests into multiple files

[jle-quel]

This PR is the first step toward restructuring the complex API.

As discussed in #8647 and #10854, the goal here is to clean the future-to-be complex API.

To simplify the review of these changes, a new PR (this PR) is created to not overload #8647.
IF/WHEN this will be approved, the second step which does the same for the marray's complex specialization will be pushed onto #8647

Here's an overview of what has changed:

For the users, the only change is the include.
Instead of including <sycl/ext/oneapi/experimental/sycl_complex.hpp>, it will now be <sycl/ext/oneapi/experimental/complex/complex.hpp>

The complex.hpp header files include all the complex/detail header file (the complex API), in order to abstract the headers needed for the users.

However, the users can include (if necessary) the specific component of the API located in complex/detail.

Here's the overview of what the complex directory will contain when the whole API is merged.

- complex
  - complex.hpp
  - detail
    - complex.hpp
    - complex_math.hpp
    - complex_group_algorithm.hpp
    - marray.hpp
    - marray_math.hpp
    - marray_group_algorithm.hpp
    - common.hpp

Finally, here's the overview of the sycl/test/extension

- complex
  - complex.cpp
  - marray.cpp

@gmlueck @steffenlarsen

[againull]

Splitting itself looks good to me. I have just one NIT concern that users will have to include header files from complex/detail directory to use specific component of API. I think we usually use detail for implementation details but in this case they contain public complex API. Maybe it's better to put them under complex directory. But I don't have strong opinion on this.
@gmlueck @steffenlarsen what do you think?

[againull]

@jle-quel It seems that several e2e tests need to updated to include complex header from the new path. Could you please take a look.

[gmlueck]

Splitting itself looks good to me. I have just one NIT concern that users will have to include header files from complex/detail directory to use specific component of API. I think we usually use detail for implementation details but in this case they contain public complex API. Maybe it's better to put them under complex directory.

We should definitely not expose "detail" to users. User-visible header files should be outside of this directory.

If you are exposing several sub-header files, are you also documenting which functions are contained in each one? Are these headers big enough to merit breaking them down into smaller units?

[jle-quel]

Thank you for your feedbacks.

First of all, I want to make sure that I understand you:

• Is this the name of the directory detail which is the problem due to the similarity with the sycl::detail namespace?
• If that's the case, there are no sycl::detail components that are exposed to the user. Strictly the name of the directory, which could be changed.

Also, users will not have to include header files from complex/detail, since complex/complex.hpp is aggregating all the subheaders. Kinda like what sycl/sycl.hpp does.
I only mentioned it because the picky user could only include the class complex and not the other components.

Lastly, where would you want to see such documentation?
This was not the case for the single header file which contained the whole API.
The extension proposal, however, does list all the entry points from this API.
Also, I thought that the name I chose for the subheader files, would be enough explicit to help the user know where each component is.

If the split looks good to you, but the directory detail name is problematic, we could change that.
If the directory itself is the problem, we could move all the subheader files one level above, so everything sits within the complex directory, and remove the complex.hpp,
however this will force the user to include every component needed instead of only one (again, like sycl/sycl.hpp)

[joeatodd]

Change in syclcompat/util.hpp LGTM

[gmlueck]

The name "detail" is the problem. We want to teach people that anything named "detail" is an implementation detail and not part of a specification.

If you do want to expose sub-headers, I'd suggest the naming pattern <sycl/complex/foo.hpp>. However, see my previous question:

Are these headers big enough to merit breaking them down into smaller units?

We should not expose this extension via a set of headers just because we can. There should be some justification for doing it this way.

The names of the headers are part of the extension API, so they must be documented in the extension specification.

[jle-quel]

Very well, how about:

- complex
    - complex.hpp // will contain the complex class, as well as the math free functions and the free group algorithms functions for complex.
    - marray.hpp  // will contain the marray class specialization, as well as the math free functions and the free group algorithms functions for marray.
    - detail // or common (if detail is not suits in this case as well)
        - common.hpp // only contains the implementation details shared by `complex.hpp` and `marray.hpp`

Even though I prefer component to be (if possible) broke down to their smaller units, it solves the breaking down into smaller units problems that you mentionned and also solves, the "detail" naming rule.

Quick note about why I prefer smaller component, when you include a header, I expect to include the strict minimum, which is something that sycl does not really do (I stumble upon doing this refacto, where including sycl/stream.hpp almost bring the whole sycl ecosystem).

Of course, this will be updated in the complex and marray, and soon to be group algorithm specification.

However, this makes the complex API fit into multiple header files.

[gmlueck]

I prefer the directory name "detail" over "common" when the directory contains implementation details (i.e. headers that the user is not supposed to include themselves).

BTW, I wasn't thinking about the extension prefix for the header file before. The full include path for the headers should be like <sycl/ext/oneapi/experimental/complex/complex.hpp>, etc.

Can you remind me which PR adds marray support to the specification? It looks like the version in the main branch (sycl_ext_oneapi_complex) does not have marray support.

[jle-quel]

I got that implicitly, the <sycl/ext/oneapi/experimental/complex/complex.hpp> 👍
The PR is this one #8852, we had some exchange on it yesterday.

Do we agree with this one (with full path) and without a single header file that includes them all?

- sycl/ext/oneapi/experimental/complex/
    - complex.hpp
    - marray.hpp
    - detail/
        - common.hpp

[gmlueck]

Do we agree with this one (with full path) and without a single header file that includes them all?

Oh, I was thinking that <complex/complex.hpp> would have all the support. Don't you think some people will want the simplicity of a single header?

[jle-quel]

So to bring back some context, @steffenlarsen and I agreed about splitting the header files since it contained multiple extensions into multiple header files.

Of course, and that's why I proposed this PR in the first place, which split everything into smaller header files (maybe too much from your feedback), and most importantly,
why I have this subdirectory which contained all the components; So that only one header file would be present under sycl/ext/oneapi/experimental/complex/

From the user's perspective, he would still have the simplicity of a single header file.
From the dev's perspective, he would have a cleaner structure of the code.

[gmlueck]

I guess I'm confused about what you are proposing, then. From a user's perspective, what header files are exposed as part of the specification? Which headers must an application include in order to use these APIs?

[bader]

Quick note about why I prefer smaller component, when you include a header, I expect to include the strict minimum, which is something that sycl does not really do (I stumble upon doing this refacto, where including sycl/stream.hpp almost bring the whole sycl ecosystem).

I totally agree. I didn't measure the cyclomatic complexity of SYCL API formally, but my gut feeling says that it's very high (backed up by my experiments with reducing the amount of code included with sycl/queue.hpp). In general, I appreciate @jle-quel attempt to split the headers into smaller independent chunks.

From the user's perspective, he would still have the simplicity of a single header file.
From the dev's perspective, he would have a cleaner structure of the code.

It also helps with the compile time, which is an issue by itself for DPC++.

[jle-quel]

I guess I'm confused about what you are proposing, then. From a user's perspective, what header files are exposed as part of the specification? Which headers must an application include in order to use these APIs?

I'm proposing an internal re-structure of the complex API.
This restructuring should not impact the user.
It should only benefit the developers.

From a user's perspective, the header file exposed to me as well as the header needed to use this API is solely: sycl/ext/oneapi/experimental/complex/complex.hpp

- complex
  - complex.hpp  // exposed and needed for the user
  - detail               // internal implementations
    - complex.hpp
    - complex_math.hpp
    - complex_group_algorithm.hpp
    - marray.hpp
    - marray_math.hpp
    - marray_group_algorithm.hpp
    - common.hpp

Now I understand that the name of the directory detail is a problem.
Would you agree to keep this layout, but change the name?

[gmlueck]

Now I understand that the name of the directory detail is a problem.
Would you agree to keeping this layout, but changing the name?

I was confused about your proposal before, and I no longer think the name "detail" is a problem. My concern is only about documenting headers in the "detail" directory. We don't want to tell users that they need to include some "detail" header. However, it seems like that's not what you are proposing. Users include "sycl/ext/oneapi/experimental/complex/complex.hpp", and that header internally includes things from "detail". That's totally fine. In fact, this is an excellent use for the name "detail".