variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes #5595

glycerine · 2020-04-25T01:55:11Z

re #5593 v1.1.0rc1, when trying to build from source on OSX 10.13.16 High Sierra, I get compilation failure:

$ mkdir build
$ cd build
$ cmake ..
$ make -j4
...
[ 72%] Building CXX object src/CMakeFiles/objxgboost.dir/metric/rank_metric.cc.o
[ 73%] Building CXX object src/CMakeFiles/objxgboost.dir/metric/survival_metric.cc.o
[ 75%] Building CXX object src/CMakeFiles/objxgboost.dir/objective/aft_obj.cc.o
[ 76%] Building CXX object src/CMakeFiles/objxgboost.dir/objective/hinge.cc.o
/my/source/github.com/dmlc/xgboost/src/metric/survival_metric.cc:80:56: error:
      variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes
        = loss_->Loss(y_lower[i], y_upper[i], yhat[i], aft_loss_distribution_scale);
                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~

 $ g++ --version
Apple LLVM version 10.0.0 (clang-1000.11.45.5)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /usr/local/bin

using libomp-10.0.0.high_sierra.bottle.tar.gz from homebrew

The text was updated successfully, but these errors were encountered:

glycerine · 2020-04-25T02:17:13Z

I have no idea if this changes correctness or not (please advise), but I get a clean build when I remove the 'const' attribute from 6 variables across src/metric/survival_metric.cc and src/objective/aft_obj.cc

diff --git a/src/metric/survival_metric.cc b/src/metric/survival_metric.cc
index 58518f00..ae299338 100644
--- a/src/metric/survival_metric.cc
+++ b/src/metric/survival_metric.cc
@@ -62,11 +62,11 @@ struct EvalAFT : public Metric {
     const auto& y_lower = info.labels_lower_bound_.HostVector();
     const auto& y_upper = info.labels_upper_bound_.HostVector();
     const auto& weights = info.weights_.HostVector();
-    const bool is_null_weight = weights.empty();
-    const float aft_loss_distribution_scale = param_.aft_loss_distribution_scale;
+    bool is_null_weight = weights.empty();
+    float aft_loss_distribution_scale = param_.aft_loss_distribution_scale;
     CHECK_LE(yhat.size(), static_cast<size_t>(std::numeric_limits<omp_ulong>::max()))
       << "yhat is too big";
-    const omp_ulong nsize = static_cast<omp_ulong>(yhat.size());
+    omp_ulong nsize = static_cast<omp_ulong>(yhat.size());
 
     double nloglik_sum = 0.0;
     double weight_sum = 0.0;
diff --git a/src/objective/aft_obj.cc b/src/objective/aft_obj.cc
index 0a5df139..7e65a1a7 100644
--- a/src/objective/aft_obj.cc
+++ b/src/objective/aft_obj.cc
@@ -47,14 +47,14 @@ class AFTObj : public ObjFunction {
     const auto& y_lower = info.labels_lower_bound_.HostVector();
     const auto& y_upper = info.labels_upper_bound_.HostVector();
     const auto& weights = info.weights_.HostVector();
-    const bool is_null_weight = weights.empty();
+    bool is_null_weight = weights.empty();
 
     out_gpair->Resize(yhat.size());
     std::vector<GradientPair>& gpair = out_gpair->HostVector();
     CHECK_LE(yhat.size(), static_cast<size_t>(std::numeric_limits<omp_ulong>::max()))
       << "yhat is too big";
-    const omp_ulong nsize = static_cast<omp_ulong>(yhat.size());
-    const float aft_loss_distribution_scale = param_.aft_loss_distribution_scale;
+    omp_ulong nsize = static_cast<omp_ulong>(yhat.size());
+    float aft_loss_distribution_scale = param_.aft_loss_distribution_scale;
 
     #pragma omp parallel for default(none) \
       firstprivate(nsize, is_null_weight, aft_loss_distribution_scale) \
@@ -73,7 +73,7 @@ class AFTObj : public ObjFunction {
   void PredTransform(HostDeviceVector<bst_float> *io_preds) override {
     // Trees give us a prediction in log scale, so exponentiate
     std::vector<bst_float> &preds = io_preds->HostVector();
-    const long ndata = static_cast<long>(preds.size()); // NOLINT(*)
+    long ndata = static_cast<long>(preds.size()); // NOLINT(*)
     #pragma omp parallel for default(none) firstprivate(ndata) shared(preds)
     for (long j = 0; j < ndata; ++j) {  // NOLINT(*)
       preds[j] = std::exp(preds[j]);

hcho3 · 2020-04-25T03:23:05Z

@glycerine Thanks for letting us know. The code compiles fine on my Macbook Pro (2019, Catalina 10.15.4), I'll look into why it is not working on High Sierra.

FYI, we currently test XGBoost using MacOS Mojave (10.14.4):

xgboost/.travis.yml

Line 9 in ef26bc4

osx_image: xcode10.3

hcho3 · 2020-04-25T03:55:20Z

@glycerine I reproduced the error by running tests inside High Sierra environment: https://travis-ci.org/github/hcho3/xgboost/jobs/679300820#L1262

hcho3 · 2020-04-25T04:12:22Z

Related: darktable-org/darktable#2189, darktable-org/darktable#3977, https://redmine.darktable.org/issues/12568#note-13

According to http://jakascorner.com/blog/2016/07/omp-default-none-and-const.html, we should not list any const variables when declaring data attributes to OpenMP loop:

diff --git a/src/metric/survival_metric.cc b/src/metric/survival_metric.cc
index 58518f00..87e4f5ee 100644
--- a/src/metric/survival_metric.cc
+++ b/src/metric/survival_metric.cc
@@ -71,7 +71,6 @@ struct EvalAFT : public Metric {
     double nloglik_sum = 0.0;
     double weight_sum = 0.0;
     #pragma omp parallel for default(none) \
-     firstprivate(nsize, is_null_weight, aft_loss_distribution_scale) \
      shared(weights, y_lower, y_upper, yhat) reduction(+:nloglik_sum, weight_sum)
     for (omp_ulong i = 0; i < nsize; ++i) {
       // If weights are empty, data is unweighted so we use 1.0 everywhere
diff --git a/src/objective/aft_obj.cc b/src/objective/aft_obj.cc
index 0a5df139..92705415 100644
--- a/src/objective/aft_obj.cc
+++ b/src/objective/aft_obj.cc
@@ -57,7 +57,6 @@ class AFTObj : public ObjFunction {
     const float aft_loss_distribution_scale = param_.aft_loss_distribution_scale;

     #pragma omp parallel for default(none) \
-      firstprivate(nsize, is_null_weight, aft_loss_distribution_scale) \
       shared(weights, y_lower, y_upper, yhat, gpair)
     for (omp_ulong i = 0; i < nsize; ++i) {
       // If weights are empty, data is unweighted so we use 1.0 everywhere
@@ -74,7 +73,7 @@ class AFTObj : public ObjFunction {
     // Trees give us a prediction in log scale, so exponentiate
     std::vector<bst_float> &preds = io_preds->HostVector();
     const long ndata = static_cast<long>(preds.size()); // NOLINT(*)
-    #pragma omp parallel for default(none) firstprivate(ndata) shared(preds)
+    #pragma omp parallel for default(none) shared(preds)
     for (long j = 0; j < ndata; ++j) {  // NOLINT(*)
       preds[j] = std::exp(preds[j]);
     }

This patch will fix compilation on High Sierra.

trivialfis · 2020-04-25T05:09:38Z

@hcho3 Your patch failed clang++-10 compilation on Linux:

/home/fis/Workspace/XGBoost/xgboost/src/metric/survival_metric.cc:77:24: error: variable 'is_null_weight' must have explicitly specified data sharing attributes
      const double w = is_null_weight ? 1.0 : weights[i];
                       ^~~~~~~~~~~~~~
/home/fis/Workspace/XGBoost/xgboost/src/metric/survival_metric.cc:73:38: note: explicit data sharing attribute requested here
    #pragma omp parallel for default(none) \
                                     ^
/home/fis/Workspace/XGBoost/xgboost/src/metric/survival_metric.cc:75:31: error: variable 'nsize' must have explicitly specified data sharing attributes
    for (omp_ulong i = 0; i < nsize; ++i) {
                              ^~~~~
/home/fis/Workspace/XGBoost/xgboost/src/metric/survival_metric.cc:73:38: note: explicit data sharing attribute requested here
    #pragma omp parallel for default(none) \
                                     ^
/home/fis/Workspace/XGBoost/xgboost/src/metric/survival_metric.cc:79:56: error: variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes
        = loss_->Loss(y_lower[i], y_upper[i], yhat[i], aft_loss_distribution_scale);
                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/fis/Workspace/XGBoost/xgboost/src/metric/survival_metric.cc:73:38: note: explicit data sharing attribute requested here
    #pragma omp parallel for default(none) \
                                     ^

I would just remove the scope constraints.

hcho3 · 2020-04-25T05:19:01Z

@trivialfis It looks like default(none) directive has unclear semantics when it comes to handling const variables. I'll remove default(none) everywhere.

hcho3 · 2020-04-25T07:38:58Z

@glycerine Fix available at #5597.

hcho3 mentioned this issue Apr 25, 2020

Fix compilation on Mac OSX High Sierra (10.13) #5597

Merged

hcho3 closed this as completed in #5597 Apr 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes #5595

variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes #5595

glycerine commented Apr 25, 2020 •

edited

Loading

glycerine commented Apr 25, 2020 •

edited by hcho3

Loading

hcho3 commented Apr 25, 2020 •

edited

Loading

hcho3 commented Apr 25, 2020

hcho3 commented Apr 25, 2020 •

edited

Loading

trivialfis commented Apr 25, 2020

hcho3 commented Apr 25, 2020

hcho3 commented Apr 25, 2020

variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes #5595

variable 'aft_loss_distribution_scale' must have explicitly specified data sharing attributes #5595

Comments

glycerine commented Apr 25, 2020 • edited Loading

glycerine commented Apr 25, 2020 • edited by hcho3 Loading

hcho3 commented Apr 25, 2020 • edited Loading

hcho3 commented Apr 25, 2020

hcho3 commented Apr 25, 2020 • edited Loading

trivialfis commented Apr 25, 2020

hcho3 commented Apr 25, 2020

hcho3 commented Apr 25, 2020

glycerine commented Apr 25, 2020 •

edited

Loading

glycerine commented Apr 25, 2020 •

edited by hcho3

Loading

hcho3 commented Apr 25, 2020 •

edited

Loading

hcho3 commented Apr 25, 2020 •

edited

Loading