Reviewer's comments.

doc/parameter.rst

@@ -405,7 +405,7 @@ Specify the learning task and the corresponding learning objective. The objectiv
 
       - When used with binary classification, the objective should be ``binary:logistic`` or similar functions that work on probability.
       - When used with multi-class classification, objective should be ``multi:softprob`` instead of ``multi:softmax``, as the latter doesn't output probability.  Also the AUC is calculated by 1-vs-rest with reference class weighted by class prevalence.
-      - When used with LTR task, the AUC is computed by comparing pairs of documents to count correctly sorted pairs.  This corresponds to pairwise learning to rank.  The implementation has some issues with average AUC around groups and distributed workers not being well ddefined.
+      - When used with LTR task, the AUC is computed by comparing pairs of documents to count correctly sorted pairs.  This corresponds to pairwise learning to rank.  The implementation has some issues with average AUC around groups and distributed workers not being well-defined.
       - On a single machine the AUC calculation is exact. In a distributed environment the AUC is a weighted average over the AUC of training rows on each node - therefore, distributed AUC is an approximation sensitive to the distribution of data across workers. Use another metric in distributed environments if precision and reproducibility are important.
       - If input dataset contains only negative or positive samples the output is `NaN`.
 

src/common/ranking_utils.cuh

@@ -63,7 +63,7 @@ SegmentedTrapezoidThreads(xgboost::common::Span<U> group_ptr,
 }
 
 /**
- * Called inside kerenl to obtain coordinate from trapezoid grid.
+ * Called inside kernel to obtain coordinate from trapezoid grid.
  */
 XGBOOST_DEVICE inline void UnravelTrapeziodIdx(size_t i_idx, size_t n,
                                                size_t *out_i, size_t *out_j) {

src/metric/auc.cc

@@ -35,7 +35,9 @@ constexpr auto UnpackArr(std::array<T, N> &&arr) {
 }
 
 /**
- * Calculate AUC for binary classification problem.
+ * Calculate AUC for binary classification problem.  This function does not normalize the
+ * AUC by 1 / (num_positive * num_negative), instead it returns a tuple for caller to
+ * handle the normalization.
  */
 std::tuple<float, float, float> BinaryAUC(std::vector<float> const &predts,
                                           std::vector<float> const &labels,
@@ -53,21 +55,21 @@ std::tuple<float, float, float> BinaryAUC(std::vector<float> const &predts,
   float label = labels[sorted_idx.front()];
   float w = get_weight(0);
   float fp = (1.0 - label) * w, tp = label * w;
-  float positive = 0, negative = 0;
+  float tp_prev = 0, fp_prev = 0;
   // TODO(jiaming): We can parallize this if we have a parallel scan for CPU.
   for (size_t i = 1; i < sorted_idx.size(); ++i) {
     if (predts[sorted_idx[i]] != predts[sorted_idx[i-1]]) {
-      auc += TrapesoidArea(negative, fp, positive, tp);
-      positive = tp;
-      negative = fp;
+      auc += TrapesoidArea(fp_prev, fp, tp_prev, tp);
+      tp_prev = tp;
+      fp_prev = fp;
     }
     label = labels[sorted_idx[i]];
     float w = get_weight(i);
     fp += (1.0f - label) * w;
     tp += label * w;
   }
 
-  auc += TrapesoidArea(negative, fp, positive, tp);
+  auc += TrapesoidArea(fp_prev, fp, tp_prev, tp);
   if (fp <= 0.0f || tp <= 0.0f) {
     auc = 0;
     fp = 0;
@@ -267,8 +269,8 @@ class EvalAUC : public Metric {
       if (tparam_->gpu_id == GenericParameter::kCpuId) {
         auc = MultiClassOVR(preds.ConstHostVector(), info);
       } else {
-        auc = GPUMultiClassAUC(preds.ConstDeviceSpan(), info, tparam_->gpu_id,
-                               &this->d_cache_);
+        auc = GPUMultiClassAUCOVR(preds.ConstDeviceSpan(), info, tparam_->gpu_id,
+                                  &this->d_cache_);
       }
     } else {
       /**
@@ -320,8 +322,8 @@ GPUBinaryAUC(common::Span<float const> predts, MetaInfo const &info,
   return std::make_tuple(0.0f, 0.0f, 0.0f);
 }
 
-float GPUMultiClassAUC(common::Span<float const> predts, MetaInfo const &info,
-                       int32_t device, std::shared_ptr<DeviceAUCCache>* cache) {
+float GPUMultiClassAUCOVR(common::Span<float const> predts, MetaInfo const &info,
+                          int32_t device, std::shared_ptr<DeviceAUCCache>* cache) {
   common::AssertGPUSupport();
   return 0;
 }

src/metric/auc.cu

@@ -45,7 +45,7 @@ struct DeviceAUCCache {
   dh::device_vector<size_t> sorted_idx;
   // track FP/TP for computation on trapesoid area
   dh::device_vector<Pair> fptp;
-  // track negative/positive for computation on trapesoid area
+  // track FP_PREV/TP_PREV for computation on trapesoid area
   dh::device_vector<Pair> neg_pos;
   // index of unique prediction values.
   dh::device_vector<size_t> unique_idx;
@@ -73,7 +73,7 @@ struct DeviceAUCCache {
  * work across threads:
  *
  * - Run scan to obtain TP/FP values, which are right coordinates of trapesoid.
- * - Find distinct prediction values and get the corresponding negative/postive value,
+ * - Find distinct prediction values and get the corresponding FP_PREV/TP_PREV value,
  *   which are left coordinates of trapesoid.
  * - Reduce the scan array into 1 AUC value.
  */
@@ -160,16 +160,16 @@ GPUBinaryAUC(common::Span<float const> predts, MetaInfo const &info,
   auto in = dh::MakeTransformIterator<float>(
       thrust::make_counting_iterator(0), [=] __device__(size_t i) {
         float fp, tp;
-        float negative, positive;
+        float fp_prev, tp_prev;
         if (i == 0) {
           // handle the last element
           thrust::tie(fp, tp) = d_fptp.back();
-          thrust::tie(negative, positive) = d_neg_pos[d_unique_idx.back()];
+          thrust::tie(fp_prev, tp_prev) = d_neg_pos[d_unique_idx.back()];
         } else {
           thrust::tie(fp, tp) = d_fptp[d_unique_idx[i] - 1];
-          thrust::tie(negative, positive) = d_neg_pos[d_unique_idx[i - 1]];
+          thrust::tie(fp_prev, tp_prev) = d_neg_pos[d_unique_idx[i - 1]];
         }
-        return TrapesoidArea(negative, fp, positive, tp);
+        return TrapesoidArea(fp_prev, fp, tp_prev, tp);
       });
 
   Pair last = cache->fptp.back();
@@ -201,8 +201,8 @@ XGBOOST_DEVICE size_t LastOf(size_t group, common::Span<Idx> indptr) {
  * MultiClass implementation is similar to binary classification, except we need to split
  * up each class in all kernels.
  */
-float GPUMultiClassAUC(common::Span<float const> predts, MetaInfo const &info,
-                       int32_t device, std::shared_ptr<DeviceAUCCache>* p_cache) {
+float GPUMultiClassAUCOVR(common::Span<float const> predts, MetaInfo const &info,
+                          int32_t device, std::shared_ptr<DeviceAUCCache>* p_cache) {
   auto& cache = *p_cache;
   if (!cache) {
     cache.reset(new DeviceAUCCache);
@@ -311,7 +311,7 @@ float GPUMultiClassAUC(common::Span<float const> predts, MetaInfo const &info,
       },
       d_fptp.size());
 
-  // scatter unique negative/positive values
+  // scatter unique FP_PREV/TP_PREV values
   auto d_neg_pos = dh::ToSpan(cache->neg_pos);
   // When dataset is not empty, each class must have at least 1 (unique) sample
   // prediction, so no need to handle special case.
@@ -343,17 +343,17 @@ float GPUMultiClassAUC(common::Span<float const> predts, MetaInfo const &info,
       thrust::make_counting_iterator(0), [=] __device__(size_t i) {
         size_t class_id = d_unique_idx[i] / n_samples;
         float fp, tp;
-        float negative, positive;
+        float fp_prev, tp_prev;
         if (i == d_unique_class_ptr[class_id]) {
           // first item is ignored, we use this thread to calculate the last item
           thrust::tie(fp, tp) = d_fptp[class_id * n_samples + (n_samples - 1)];
-          thrust::tie(negative, positive) =
+          thrust::tie(fp_prev, tp_prev) =
               d_neg_pos[d_unique_idx[LastOf(class_id, d_unique_class_ptr)]];
         } else {
           thrust::tie(fp, tp) = d_fptp[d_unique_idx[i] - 1];
-          thrust::tie(negative, positive) = d_neg_pos[d_unique_idx[i - 1]];
+          thrust::tie(fp_prev, tp_prev) = d_neg_pos[d_unique_idx[i - 1]];
         }
-        float auc = TrapesoidArea(negative, fp, positive, tp);
+        float auc = TrapesoidArea(fp_prev, fp, tp_prev, tp);
         return auc;
       });
 

src/metric/auc.h

@@ -25,8 +25,8 @@ std::tuple<float, float, float>
 GPUBinaryAUC(common::Span<float const> predts, MetaInfo const &info,
              int32_t device, std::shared_ptr<DeviceAUCCache> *p_cache);
 
-float GPUMultiClassAUC(common::Span<float const> predts, MetaInfo const &info,
-                       int32_t device, std::shared_ptr<DeviceAUCCache>* cache);
+float GPUMultiClassAUCOVR(common::Span<float const> predts, MetaInfo const &info,
+                          int32_t device, std::shared_ptr<DeviceAUCCache>* cache);
 
 std::pair<float, uint32_t>
 GPURankingAUC(common::Span<float const> predts, MetaInfo const &info,