Add Doxygen CI check and pre-commit hook

[PointKernel]

This work is mainly taken from rapidsai/cudf#11076.

It adds Doxygen CI check and pre-commit hook. All the existing Doxygen warnings are fixed.

[karthikeyann]

Great work!
ci/checks/style.sh runs only clang-format. If you would like to run doxygen check in ci, style.sh needs to be updated.

Also, is there a plan to publish doxygen documentation in github.io pages?

[PointKernel]

If you would like to run doxygen check in ci, style.sh needs to be updated.

I noticed in the cudf style.sh there is no command invoking doxygen.sh thus wonder how doxygen check is triggered in CI.

is there a plan to publish doxygen documentation in github.io pages?

Nothing solid yet but yes, it's on my to-do list. Any comments here?

[karthikeyann]

I noticed in the cudf style.sh there is no command invoking doxygen.sh thus wonder how doxygen check is triggered in CI.

https://github.com/rapidsai/cudf/blob/4bbb65876d1d775a8523e416fddae0d805caefb0/ci/checks/style.sh#L23
cudf runs all pre-commit hooks against all files.
pre-commit run --hook-stage manual --all-files

is there a plan to publish doxygen documentation in github.io pages?

Nothing solid yet but yes, it's on my to-do list. Any comments here?

Depends on when documentation should be updated. I don't see releases or tags in this repo.
How about updating documentation on every push to main branch? (last updated July 16, 2020 ! not update for long time!)

[PointKernel]

cudf runs all pre-commit hooks against all files.

Great, I've updated the cuco script to do the same.

@karthikeyann Can you please re-review? Also, thanks for filing #178 to track the doc automation separately.

[karthikeyann]

Fix for remaining doxygen warnings as patch.
(I don't have write permissions to this repo, I could not push my commit to this PR)

From 63635ee3f967e82b444376b8eab4a56462ace388 Mon Sep 17 00:00:00 2001
From: Karthikeyan Natarajan <[email protected]>
Date: Tue, 21 Jun 2022 10:47:20 +0530
Subject: [PATCH 1/1] fix some doxygen warnings

---
 include/cuco/detail/probe_sequence_impl.cuh | 27 ++++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/cuco/detail/probe_sequence_impl.cuh b/include/cuco/detail/probe_sequence_impl.cuh
index 6fdb32b..c7ad07b 100644
--- a/include/cuco/detail/probe_sequence_impl.cuh
+++ b/include/cuco/detail/probe_sequence_impl.cuh
@@ -47,7 +47,7 @@ class probe_sequence_base {
   static constexpr uint32_t vector_width() noexcept { return 2u; }
 };

-/*
+/**
  * @brief Base class of probe sequence implementation.
  *
  * Hash map operations are generally memory-bandwidth bound. A vector-load loads two consecutive
@@ -70,14 +70,17 @@ template <typename Key,
           uint32_t CGSize>
 class probe_sequence_impl_base {
  protected:
-  using value_type         = cuco::pair_type<Key, Value>;
-  using key_type           = Key;
-  using mapped_type        = Value;
-  using atomic_key_type    = cuda::atomic<key_type, Scope>;
-  using atomic_mapped_type = cuda::atomic<mapped_type, Scope>;
-  using pair_atomic_type   = cuco::pair_type<atomic_key_type, atomic_mapped_type>;
-  using iterator           = pair_atomic_type*;
-  using const_iterator     = pair_atomic_type const*;
+  using value_type         = cuco::pair_type<Key, Value>;       ///< Type of key/value pairs
+  using key_type           = Key;                               ///< Key type
+  using mapped_type        = Value;                             ///< Type of mapped values
+  using atomic_key_type    = cuda::atomic<key_type, Scope>;     ///< Type of atomic keys
+  using atomic_mapped_type = cuda::atomic<mapped_type, Scope>;  ///< Type of atomic mapped values
+  /// Pair type of atomic key and atomic mapped value
+  using pair_atomic_type = cuco::pair_type<atomic_key_type, atomic_mapped_type>;
+  /// Type of the forward iterator to `pair_atomic_type`
+  using iterator = pair_atomic_type*;
+  /// Type of the forward iterator to `const pair_atomic_type`
+  using const_iterator = pair_atomic_type const*;

   /**
    * @brief Returns the number of elements loaded with each vector-load.
@@ -115,6 +118,8 @@ class probe_sequence_impl_base {
  public:
   /**
    * @brief Returns the capacity of the hash map.
+   *
+   * @return The capacity of the hash map.
    */
   __host__ __device__ __forceinline__ std::size_t get_capacity() const noexcept
   {
@@ -123,11 +128,15 @@ class probe_sequence_impl_base {

   /**
    * @brief Returns slots array.
+   *
+   * @return Slots array
    */
   __device__ __forceinline__ iterator get_slots() noexcept { return slots_; }

   /**
    * @brief Returns slots array.
+   *
+   * @return Slots array
    */
   __device__ __forceinline__ const_iterator get_slots() const noexcept { return slots_; }

--
2.36.1

[karthikeyann]

LGTM 👍

[jrhemstad]

What's the deal with <tt>? afaik, markdown backticks work in Doxygen.

[PointKernel]

Here's an example warning message of using backticks https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/nvidia%2Fgpuci%2FcuCollections%2Fprb%2FcuCollections-style/detail/cuCollections-style/147/pipeline#log-176. This happened exclusively to all docs containing the keyword "value_type".

[jrhemstad]

Which error in particular should I be looking at?

[PointKernel]

warning: explicit link request to 'value_type' could not be resolved

[PointKernel]

Here's what I found:

* `std::iterator_traits<InputIt>`

Works fine but adding ::value_type will fail with explicit link request to 'value_type' could not be resolved:

* `std::iterator_traits<InputIt>::value_type`

I thought the problem is that we didn't provide a link to the implementation of "std::iterator_traits" thus the link request to the code fails. So maybe marking a locally defined function with backticks will pass? Following this idea, I tried the below code but had no luck either:

* `cuco::detail::Murmurhash3_32<key_type>::result_type`

Another thing I tried is turning off AUTOLINK_SUPPORT in the doxygen config file. We can use backticks without getting any warnings by doing that but I think our intention is to keep the auto-link on in Doxygen. Any thoughts?

[jrhemstad]

I suspect it's expecting docs to exist for std::iterator_traits<InputIt>::value_type to exist and link to or something.

[jrhemstad]

I'm excited for this!

I'd like to expand the Doxygen section of the README with an example of what our expectations/conventions are for documentation (we can steal this from libcudf).

The change from__forceinline__ to inline seems orthogonal to this PR.

I also am not a fan of having to use <tt></tt> for code font. Whenever I've checked in the past, markdown ticks work in Doxygen.