[REVIEW] Clean Up #include Dependencies

[mdemoret-nv]

Closes #3376

This PR moves a few header files around to fix bad dependencies from include -> src. Moving forward, the only allowed #include direction will be:

• src -> include
• src -> src_prims
• src_prims -> include

To facilitate this, a few changes needed to be made:

1. Functions for the C-API have been separated into their own *_api.cpp file (some were combined with C++ files)
2. host_buffer, device_buffer, hostAllocator and deviceAllocator were moved from src_prims to include
3. #include <common/cumlHandle.hpp> has been removed from all C++ files.

This PR includes some additional improvements:

• Updated the include_checker.py script:
  • Added functionality to check for badly placed #pragma once
  • Added functionality to fix some of the existing warnings
  • General refactor to support more checks
• Fixed all incorrect uses of #pragma once
• Fixed incorrect uses of using namespace ... in header files outside of a namespace
• Removed some unnecessary #include

While this touches a lot of files, the actual number of changes is relatively small. Below is a before/after comparison of the include graphs:

src/include:
Before:

After:

src/src_prims:
Before:

After:

[JohnZed]

I read the python and cmakelists files in some detail, but went pretty quickly over the headers. Headers look good. Just a couple small changes to check out. Let's discuss timing separately.

[teju85]

It is finally great to see our C-API also evolving!

[mdemoret-nv]

After reviewing @teju85 comments, I've updated the C-API structure a bit and added back in the #ifdef __cplusplus to the C-API headers. I was under the impression we were actually building the C-API with gcc which would make these #ifdefs moot. However, it looks like we are actually using g++ and relying on the extern "C" to not mangle the names. This was interesting to discover since it was very unlikely any C program could actually use the C-API headers in their previous state.

To ensure the C-API can actually be used, I added a simple test library that is compiled only in C and imports libcuml.so. This flushed out a lot of errors that would have prevented anyone from using the C-API.

I would appreciate a re-review on the updated changes if possible. I still believe this PR is low ris because most changes are to #include statements which allows us to rely on the compiler to ensure everything is correct.

[dantegd]

Just left 2 suggestions but changes, particularly the new C api tests look great!

[JohnZed]

Looks great, just one question (and one tiny change). You flipped a couple of includes from absolute to relative and others from relative to absolute. How should we think about the simplest rule for when to prefer absolute or relative #includes?

[mdemoret-nv]

Looks great, just one question (and one tiny change). You flipped a couple of includes from absolute to relative and others from relative to absolute. How should we think about the simplest rule for when to prefer absolute or relative #includes?

@JohnZed The changes between relative and absolute were due to the script include_checker.py which has an --inplace option. Relative includes should only be used if you jump <2 directories in a single direction. Everything else should be absolute. For example:

• #include ../../my_header.hpp
  • This is ok because we only jump 2 directories
• #include ../distance/my_header.hpp
  • This should be avoided (and will be corrected by the script) because we are going up one and over one. Convert to absolute.

While these rules are the ones the include_checker.py script will change, I have been working on a comprehensive set of rules that we should adopt as part of a larger rework of the include structure (that Will and I have been looking at to improve compilation speed). Between the include_checker.py script and clang-format version 10, almost all of this can be automated and could be run in the style check

[mdemoret-nv]

rerun tests

[teju85]

Yes, @mdemoret-nv you are right. For the libraries which expose a C-API, but use g++ (for whatever reasons), we'll have to put these __cplusplus guards. Thank you for fixing it.

Really appreciate you adding a C-API testing in here!

LGTM.

[JohnZed]

@gpucibot merge