Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenMP detection is still broken on MacOS #8946

Open
barracuda156 opened this issue Mar 19, 2023 · 26 comments
Open

OpenMP detection is still broken on MacOS #8946

barracuda156 opened this issue Mar 19, 2023 · 26 comments

Comments

@barracuda156
Copy link

Despite an earlier PR claiming to fix it: #8684
It is still broken, at least with GCC:

checking whether OpenMP will work in a package... no
*****************************************************************************************
         OpenMP is unavailable on this Mac OSX system. Training speed may be suboptimal.
         To use all CPU cores for training jobs, you should install OpenMP by running\n
             brew install libomp

Moreover, it gives a wrong recommendation (libomp is Clang-specific).

P. S. IMO it is a bad wording to use “should“ for an optional package manager. One does not need any – libomp, while not needed at all with GCC, can be installed directly. Also, there is Macports which has it, not just Homebrew.

@barracuda156
Copy link
Author

Example of configure that actually works: 2005m/kit#36 (comment)
(Prefix gotta be adjusted for the package manager being used.)

@jameslamb
Copy link
Contributor

jameslamb commented Mar 19, 2023

Thanks for tagging me and for the additional details. Are you interested in using your knowledge of this area to improve the project by opening a pull request? If not @trivialfis I'd be happy to look into this in a few days.

@barracuda156
Copy link
Author

Thanks for tagging me and for the additional details. Are you interested inusing your knowledge of this area to improve the prpject by opening a pull request? If not @trivialfis I'd be happy to look into this in a few days.

@jameslamb Thank you for responding! I am interested to have this fixed so that we can have xgboost for R in Macports. Allow me a day, perhaps, I need to make sure build is fixed (which includes 32-bit) and tests pass, and then figure out how to amend OpenMP checks so that they work for a) GCC and Clang and b) Homebrew and Macports cases.

@trivialfis
Copy link
Member

Thank you for raising an issue! As suggested by @jameslamb , would be great if you can submit a fix, your expertise in this area is really appreciated.

@hcho3
Copy link
Collaborator

hcho3 commented Mar 19, 2023

@barracuda156 Thanks for raising the issue. As you noticed, currently XGBoost has only been tested with the Apple Clang + Homebrew combination. It will be great if you can make it work with Macports.

@barracuda156
Copy link
Author

barracuda156 commented Mar 20, 2023

UPD. Tests mostly pass, there is one failure on ppc32, which is a known problem, though it is not immediately obvious how to fix it in this case (malloc errors usually happen with executables; those are caused by duplicate libstdc++ in older macOS with modern GCC installed):

R version 4.2.2 (2022-10-31) -- "Innocent and Trusting"
Copyright (C) 2022 The R Foundation for Statistical Computing
Platform: powerpc-apple-darwin10.8.0 (32-bit)

> library(testthat)
> library(xgboost)
> 
> test_check("xgboost", reporter = ProgressReporter)
✔ | F W S  OK | Context

⠏ |         0 | basic                                                           
⠏ |         0 | basic functions                                                 
⠋ |         1 | basic functions                                                 
⠸ |         4 | basic functions                                                 [15:43:36] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠇ |         9 | basic functions                                                 
⠹ |        13 | basic functions                                                 [1]	train-rmse:1.678965 
[2]	train-rmse:1.678965 
[3]	train-rmse:1.524624 
[4]	train-rmse:1.441523 
[5]	train-rmse:1.437758 
[6]	train-rmse:1.377895 
[7]	train-rmse:1.305893 
[8]	train-rmse:1.336120 
[9]	train-rmse:1.316072 
[10]	train-rmse:1.316031 
[11]	train-rmse:1.319404 
[12]	train-rmse:1.235097 
[13]	train-rmse:1.225430 
[14]	train-rmse:1.221079 
[15]	train-rmse:1.235220 
[16]	train-rmse:1.219153 
[17]	train-rmse:1.226439 
[18]	train-rmse:1.235026 
[19]	train-rmse:1.244651 
[20]	train-rmse:1.253197 
[21]	train-rmse:1.262494 
[22]	train-rmse:1.272865 
[23]	train-rmse:1.276073 
[24]	train-rmse:1.277040 
[25]	train-rmse:1.287974 
[26]	train-rmse:1.289816 
[27]	train-rmse:1.292365 
[28]	train-rmse:1.282785 
[29]	train-rmse:1.283805 
[30]	train-rmse:1.290661 
[31]	train-rmse:1.289439 
[32]	train-rmse:1.281995 
[15:43:36] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠸ |        14 | basic functions                                                 [15:43:36] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠴ |        16 | basic functions                                                 
⠇ |        19 | basic functions                                                 [15:43:37] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠦ |        27 | basic functions                                                 
⠇ |        29 | basic functions                                                 
⠼ |        35 | basic functions                                                 [1]	train-error:0.028405 

⠦ |        37 | basic functions                                                 [15:43:37] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠏ |        40 | basic functions                                                 
⠙ |        42 | basic functions                                                 [15:43:37] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠦ |        47 | basic functions                                                 
⠋ |        51 | basic functions                                                 
⠴ |        56 | basic functions                                                 [1]	train-logloss:0.439409 
[2]	train-logloss:0.299260 
[3]	train-logloss:0.209937 
[4]	train-logloss:0.150151 

⠇ |        59 | basic functions                                                 
⠏ |        60 | basic functions                                                 
⠏ |        70 | basic functions                                                 [1]	train-logloss:0.233470+0.001565	test-logloss:0.233607+0.004930 
[2]	train-logloss:0.136851+0.001767	test-logloss:0.137010+0.006645 
[1]	train-logloss:0.233482+0.001761	test-logloss:0.233527+0.005591 
[2]	train-logloss:0.136860+0.002158	test-logloss:0.136933+0.007299 

⠋ |        71 | basic functions                                                 
⠙ |        72 | basic functions                                                 
⠹ |        73 | basic functions                                                 
⠸ |        74 | basic functions                                                 
⠦ |        77 | basic functions                                                 
⠧ |        78 | basic functions                                                 
⠏ |        80 | basic functions                                                 [1]	train-logloss:0.380598 
[2]	train-logloss:0.247331 
[3]	train-logloss:0.175047 
[4]	train-logloss:0.122301 
[5]	train-logloss:0.089889 
[1]	train-logloss:0.497338 
[2]	train-logloss:0.357306 
[3]	train-logloss:0.257215 
[4]	train-logloss:0.184518 
[5]	train-logloss:0.132113 

⠙ |        82 | basic functions                                                 
⠸ |        84 | basic functions                                                 [1]	train-error:0.046522	train-auc:0.958228	train-logloss:0.233376 
[2]	train-error:0.022263	train-auc:0.981413	train-logloss:0.136658 

⠼ |        85 | basic functions                                                 [1]	train-merror:0.040000 
[2]	train-merror:0.026667 

⠇ |        89 | basic functions                                                 [1]	train-error:0.046522	train-auc:0.958228	train-logloss:0.482541 
[2]	train-error:0.046522	train-auc:0.987161	train-logloss:0.359536 
R(48275,0xa0dfb620) malloc: *** mmap(size=840388608) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
R(48275,0xa0dfb620) malloc: *** mmap(size=840388608) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug

⠸ | 1      93 | basic functions                                                 
⠼ | 1      94 | basic functions                                                 
✖ | 1      95 | basic functions [21.9s]
────────────────────────────────────────────────────────────────────────────────
Error ('test_basic.R:461:3'): strict_shape works
Error: cannot allocate vector of size 801.5 Mb
Backtrace:
 1. xgboost (local) test_agaricus()
      at test_basic.R:461:2
 2. xgboost (local) test_strict_shape(bst, X, 1)
      at test_basic.R:457:4
 4. xgboost:::predict.xgb.Booster(...)
 5. base::array(data = ret, dim = rev(shape))
────────────────────────────────────────────────────────────────────────────────

⠏ |         0 | callbacks                                                       
⠏ |         0 | callbacks                                                       
⠋ |         1 | callbacks                                                       [1]	train-auc:0.900000	test-auc:0.800000 

⠙ |        12 | callbacks                                                       
⠴ |        26 | callbacks                                                       
⠏ |        30 | callbacks                                                       
⠙ |        32 | callbacks                                                       
⠸ |        34 | callbacks                                                       
⠴ |        36 | callbacks                                                       
⠧ |        38 | callbacks                                                       
⠇ |        39 | callbacks                                                       
⠋ |        41 | callbacks                                                       
⠹ |        43 | callbacks                                                       
⠇ |   1    48 | callbacks                                                       
⠙ |   1    51 | callbacks                                                       
⠧ |   1    57 | callbacks                                                       
⠹ |   1    62 | callbacks                                                       [15:44:00] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[1]	train-auc:0.829749 
Will train until train_auc hasn't improved in 3 rounds.

[2]	train-auc:0.829749 
[3]	train-auc:0.829749 
[4]	train-auc:0.829749 
Stopping. Best iteration:
[1]	train-auc:0.829749


⠇ |   1    68 | callbacks                                                       
⠏ |   1    69 | callbacks                                                       
⠼ |   1    74 | callbacks                                                       
⠇ |   1    78 | callbacks                                                       
⠙ |   1    81 | callbacks                                                       
⠼ |   1    84 | callbacks                                                       
⠹ |   1    92 | callbacks                                                       
✔ |   1    95 | callbacks [7.4s]
────────────────────────────────────────────────────────────────────────────────
Warning ('test_callbacks.R:196:3'): cb.save.model works as expected
one argument not used by format 'xgboost.json'
Backtrace:
 1. xgboost::xgb.train(...)
      at test_callbacks.R:196:2
 2. xgboost (local) f()
 4. base::sprintf(save_name, env$iteration)
────────────────────────────────────────────────────────────────────────────────

⠏ |         0 | config                                                          
⠏ |         0 | Test global configuration                                       
✔ |         8 | Test global configuration

⠏ |         0 | custom_objective                                                
⠏ |         0 | Test models with custom objective                               [1]	eval-error:0.042831	train-error:0.046522 
[2]	eval-error:0.021726	train-error:0.022263 

⠋ |         1 | Test models with custom objective                               
⠼ |         5 | Test models with custom objective                               [1]	eval-error:0.042831	train-error:0.046522 
[2]	eval-error:0.021726	train-error:0.022263 
[3]	eval-error:0.018001	train-error:0.015200 
[4]	eval-error:0.018001	train-error:0.015200 
[5]	eval-error:0.006207	train-error:0.007063 
[6]	eval-error:0.000000	train-error:0.001228 
[7]	eval-error:0.000000	train-error:0.001228 
[8]	eval-error:0.000000	train-error:0.001228 
[9]	eval-error:0.000000	train-error:0.001228 
[10]	eval-error:0.000000	train-error:0.000000 

⠧ |         8 | Test models with custom objective                               [1]	eval-error:0.042831	train-error:0.046522 
[2]	eval-error:0.021726	train-error:0.022263 

⠋ |        11 | Test models with custom objective                               
✔ |        12 | Test models with custom objective [1.8s]

⠏ |         0 | dmatrix                                                         
⠏ |         0 | testing xgb.DMatrix functionality                               
⠴ |         6 | testing xgb.DMatrix functionality                               
⠏ |        10 | testing xgb.DMatrix functionality                               [15:44:07] 6513x126 matrix with 143286 entries loaded from /opt/local/var/macports/build/_opt_PPCRosettaPorts_R_R-xgboost/R-xgboost/work/.tmp/RtmpDTgQsS/xgb.DMatrix_bc934c89f2d4

⠋ |        11 | testing xgb.DMatrix functionality                               
⠋ |        21 | testing xgb.DMatrix functionality                               
⠴ |   1    25 | testing xgb.DMatrix functionality                               
⠹ |   1    32 | testing xgb.DMatrix functionality                               
⠼ |   1    34 | testing xgb.DMatrix functionality                               
✔ |   1    40 | testing xgb.DMatrix functionality [1.2s]
────────────────────────────────────────────────────────────────────────────────
Warning ('test_dmatrix.R:82:3'): xgb.DMatrix: getinfo & setinfo
NAs introduced by coercion
Backtrace:
 1. testthat::expect_error(setinfo(dtest, "weight", rep("a", nrow(test_data))))
      at test_dmatrix.R:82:2
 7. xgboost:::setinfo.xgb.DMatrix(dtest, "weight", rep("a", nrow(test_data)))
────────────────────────────────────────────────────────────────────────────────

⠏ |         0 | feature_weights                                                 
⠏ |         0 | feature weights                                                 
⠋ |         1 | feature weights                                                 
⠹ |         3 | feature weights                                                 
⠼ |         5 | feature weights                                                 
✔ |         6 | feature weights [1.0s]

⠏ |         0 | gc_safety                                                       
⠏ |         0 | Garbage Collection Safety Check                                 [1]	train-logloss:0.233376 
[2]	train-logloss:0.136658 

⠋ |         1 | Garbage Collection Safety Check                                 
✔ |         1 | Garbage Collection Safety Check [80.4s]

⠏ |         0 | glm                                                             
⠏ |         0 | Test generalized linear models                                  
⠋ |         1 | Test generalized linear models                                  
⠴ |         6 | Test generalized linear models                                  
⠧ |         8 | Test generalized linear models                                  
⠋ |        11 | Test generalized linear models                                  [1]	eval-error:0.002483	train-error:0.004760 
Multiple eval metrics are present. Will use train_error for early stopping.
Will train until train_error hasn't improved in 1 rounds.

[2]	eval-error:0.000621	train-error:0.002610 
[3]	eval-error:0.000000	train-error:0.001842 
[4]	eval-error:0.000000	train-error:0.001228 
[5]	eval-error:0.000000	train-error:0.000614 
[6]	eval-error:0.000000	train-error:0.000614 
Stopping. Best iteration:
[5]	eval-error:0.000000	train-error:0.000614


⠙ |        12 | Test generalized linear models                                  [1]	eval-error:0.002483	train-error:0.004760 
Multiple eval metrics are present. Will use train_error for early stopping.
Will train until train_error hasn't improved in 1 rounds.

[2]	eval-error:0.000621	train-error:0.002610 
[3]	eval-error:0.000000	train-error:0.001842 
[4]	eval-error:0.000000	train-error:0.001228 
[5]	eval-error:0.000000	train-error:0.000614 
[6]	eval-error:0.000000	train-error:0.000614 
Stopping. Best iteration:
[5]	eval-error:0.000000	train-error:0.000614


✔ |        13 | Test generalized linear models [1.2s]

⠏ |         0 | helpers                                                         
⠏ |         0 | Test helper functions                                           
⠋ |         1 | Test helper functions                                           [1]	train-logloss:0.636592 

⠙ |        12 | Test helper functions                                           
⠴ |        26 | Test helper functions                                           
⠋ |        31 | Test helper functions                                           [1]	train-rmse:1.940066 
[2]	train-rmse:1.560864 
[3]	train-rmse:1.277414 
[4]	train-rmse:1.068562 
[5]	train-rmse:0.903897 
[6]	train-rmse:0.763228 
[7]	train-rmse:0.664924 
[8]	train-rmse:0.570358 
[9]	train-rmse:0.507416 
[10]	train-rmse:0.458599 
[11]	train-rmse:0.394349 
[12]	train-rmse:0.355794 
[13]	train-rmse:0.305484 
[14]	train-rmse:0.271176 
[15]	train-rmse:0.259943 
[16]	train-rmse:0.238429 
[17]	train-rmse:0.228003 
[18]	train-rmse:0.212117 
[19]	train-rmse:0.187830 
[20]	train-rmse:0.173381 
[21]	train-rmse:0.164001 
[22]	train-rmse:0.156113 
[23]	train-rmse:0.143242 
[24]	train-rmse:0.130215 
[25]	train-rmse:0.120160 
[26]	train-rmse:0.112119 
[27]	train-rmse:0.104753 
[28]	train-rmse:0.096786 
[29]	train-rmse:0.089361 
[30]	train-rmse:0.083706 

⠴ |        46 | Test helper functions                                           
⠋ |        71 | Test helper functions                                           
⠇ |        99 | Test helper functions                                           
⠴ |       126 | Test helper functions                                           
⠏ |       150 | Test helper functions                                           
⠇ |       179 | Test helper functions                                           
⠹ |       203 | Test helper functions                                           
⠧ |       228 | Test helper functions                                           [1]	train-rmse:1.940066 
[2]	train-rmse:1.675038 
[3]	train-rmse:1.462383 
[4]	train-rmse:1.283198 
[5]	train-rmse:1.155542 
[6]	train-rmse:1.049559 
[7]	train-rmse:0.942910 
[8]	train-rmse:0.859371 
[9]	train-rmse:0.774970 
[10]	train-rmse:0.725452 
[11]	train-rmse:0.679127 
[12]	train-rmse:0.628614 
[13]	train-rmse:0.594549 
[14]	train-rmse:0.535545 
[15]	train-rmse:0.485624 
[16]	train-rmse:0.460187 
[17]	train-rmse:0.413632 
[18]	train-rmse:0.403692 
[19]	train-rmse:0.385314 
[20]	train-rmse:0.366653 
[21]	train-rmse:0.354532 
[22]	train-rmse:0.326526 
[23]	train-rmse:0.316875 
[24]	train-rmse:0.301910 
[25]	train-rmse:0.285458 
[26]	train-rmse:0.275437 
[27]	train-rmse:0.267556 
[28]	train-rmse:0.263727 
[29]	train-rmse:0.253460 
[30]	train-rmse:0.235433 

⠧ |       248 | Test helper functions                                           
⠸ |       274 | Test helper functions                                           
⠏ |       300 | Test helper functions                                           
⠼ |       325 | Test helper functions                                           
⠙ |       352 | Test helper functions                                           
⠇ |       379 | Test helper functions                                           
⠴ |       406 | Test helper functions                                           
⠼ |       435 | Test helper functions                                           
⠙ |       452 | Test helper functions                                           
⠸ |       464 | Test helper functions                                           
⠇ |       479 | Test helper functions                                           
⠼ |       495 | Test helper functions                                           
⠏ |       510 | Test helper functions                                           
⠴ |       526 | Test helper functions                                           
⠙ |       542 | Test helper functions                                           
⠧ |       558 | Test helper functions                                           
⠼ |       575 | Test helper functions                                           
⠋ |       591 | Test helper functions                                           
⠦ |       607 | Test helper functions                                           
⠙ |       622 | Test helper functions                                           
⠇ |       639 | Test helper functions                                           
⠴ |       656 | Test helper functions                                           
⠼ |       665 | Test helper functions                                           
⠧ |       668 | Test helper functions                                           
⠏ |       670 | Test helper functions                                           
⠹ |       673 | Test helper functions                                           
⠏ |       680 | Test helper functions                                           
⠋ |       681 | Test helper functions                                           
⠙ |       682 | Test helper functions                                           [1]	train-rmse:0.943398 

⠸ |       684 | Test helper functions                                           
⠋ |       691 | Test helper functions                                           
⠴ |       696 | Test helper functions                                           
⠦ |       697 | Test helper functions                                           
⠧ |     1 697 | Test helper functions                                           
⠇ |     1 698 | Test helper functions                                           
⠏ |     1 699 | Test helper functions                                           
⠦ |     1 706 | Test helper functions                                           
⠹ |     1 712 | Test helper functions                                           
⠸ |     1 713 | Test helper functions                                           
⠼ |     1 714 | Test helper functions                                           
⠧ |     1 717 | Test helper functions                                           
⠴ |     1 745 | Test helper functions                                           
✔ |     1 745 | Test helper functions [16.5s]
────────────────────────────────────────────────────────────────────────────────
Skip ('test_helpers.R:386:1'): xgb.plot.multi.trees works with and without feature names
Reason: empty test
────────────────────────────────────────────────────────────────────────────────

⠏ |         0 | interaction_constraints                                         
⠏ |         0 | interaction constraints                                         
⠋ |         1 | interaction constraints                                         
⠙ |         2 | interaction constraints                                         
✔ |         2 | interaction constraints [29.9s]

⠏ |         0 | interactions                                                    
⠏ |         0 | Test prediction of feature interactions                         
⠋ |         1 | Test prediction of feature interactions                         
⠸ |         4 | Test prediction of feature interactions                         
⠼ |         5 | Test prediction of feature interactions                         
⠋ |        11 | Test prediction of feature interactions                         
⠹ |        13 | Test prediction of feature interactions                         
⠼ |        15 | Test prediction of feature interactions                         [1]	train-logloss:0.482541 
[2]	train-logloss:0.359536 
[3]	train-logloss:0.279935 
[4]	train-logloss:0.218599 

⠧ |        18 | Test prediction of feature interactions                         
✔ |        19 | Test prediction of feature interactions [9.0s]

⠏ |         0 | io                                                              
⠏ |         0 | Test model IO.                                                  [1]	train-logloss:0.439409 
[2]	train-logloss:0.299260 
[3]	train-logloss:0.209937 
[4]	train-logloss:0.150151 
[5]	train-logloss:0.108673 
[6]	train-logloss:0.079348 
[7]	train-logloss:0.058385 
[8]	train-logloss:0.043147 

⠋ |         1 | Test model IO.                                                  
✔ |         2 | Test model IO. [0.5s]

⠏ |         0 | model_compatibility                                             
⠏ |         0 | Models from previous versions of XGBoost can be loaded          
⠋ |         1 | Models from previous versions of XGBoost can be loaded          
⠦ |         7 | Models from previous versions of XGBoost can be loaded          
⠧ |         8 | Models from previous versions of XGBoost can be loaded          
⠋ |        21 | Models from previous versions of XGBoost can be loaded          
⠼ |        35 | Models from previous versions of XGBoost can be loaded          
⠧ |        48 | Models from previous versions of XGBoost can be loaded          
⠋ |        61 | Models from previous versions of XGBoost can be loaded          
⠼ |        75 | Models from previous versions of XGBoost can be loaded          
⠏ |        90 | Models from previous versions of XGBoost can be loaded          
⠹ |       103 | Models from previous versions of XGBoost can be loaded          
⠴ |       116 | Models from previous versions of XGBoost can be loaded          
⠴ |       126 | Models from previous versions of XGBoost can be loaded          
⠹ |       143 | Models from previous versions of XGBoost can be loaded          
⠴ |       156 | Models from previous versions of XGBoost can be loaded          
⠼ |       165 | Models from previous versions of XGBoost can be loaded          
⠧ |       178 | Models from previous versions of XGBoost can be loaded          
⠏ |       190 | Models from previous versions of XGBoost can be loaded          
⠋ |       201 | Models from previous versions of XGBoost can be loaded          
⠏ |       210 | Models from previous versions of XGBoost can be loaded          
⠋ |       221 | Models from previous versions of XGBoost can be loaded          
✔ |       233 | Models from previous versions of XGBoost can be loaded [5.9s]

⠏ |         0 | monotone                                                        
⠏ |         0 | monotone constraints                                            
⠋ |         1 | monotone constraints                                            
✔ |         1 | monotone constraints [0.2s]

⠏ |         0 | parameter_exposure                                              
⠏ |         0 | Test model params and call are exposed to R                     
⠋ |         1 | Test model params and call are exposed to R                     
✔ |         6 | Test model params and call are exposed to R [0.3s]

⠏ |         0 | poisson_regression                                              
⠏ |         0 | Test Poisson regression model                                   
✔ |         3 | Test Poisson regression model

⠏ |         0 | ranking                                                         
⠏ |         0 | Learning to rank                                                [1]	train-auc:0.575000	train-aucpr:0.550000 
[2]	train-auc:0.650000	train-aucpr:0.700000 
[3]	train-auc:0.725000	train-aucpr:0.850000 
[4]	train-auc:0.800000	train-aucpr:1.000000 
[5]	train-auc:0.800000	train-aucpr:1.000000 
[6]	train-auc:0.800000	train-aucpr:1.000000 
[7]	train-auc:0.800000	train-aucpr:1.000000 
[8]	train-auc:0.800000	train-aucpr:1.000000 
[9]	train-auc:0.800000	train-aucpr:1.000000 
[10]	train-auc:0.800000	train-aucpr:1.000000 
[1]	train-auc:0.575000	train-aucpr:0.550000 
[2]	train-auc:0.650000	train-aucpr:0.700000 
[3]	train-auc:0.725000	train-aucpr:0.850000 
[4]	train-auc:0.725000	train-aucpr:0.850000 
[5]	train-auc:0.725000	train-aucpr:0.850000 
[6]	train-auc:0.725000	train-aucpr:0.850000 
[7]	train-auc:0.725000	train-aucpr:0.850000 
[8]	train-auc:0.800000	train-aucpr:1.000000 
[9]	train-auc:0.800000	train-aucpr:1.000000 
[10]	train-auc:0.800000	train-aucpr:1.000000 

⠹ |         3 | Learning to rank                                                [15:46:33] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:33] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

⠇ |         9 | Learning to rank                                                [15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.
[15:46:34] WARNING: src/c_api/c_api.cc:935: `ntree_limit` is deprecated, use `iteration_range` instead.

✔ |        14 | Learning to rank [0.3s]

⠏ |         0 | update                                                          
⠏ |         0 | update trees in an existing model                               [15:46:34] WARNING: src/gbm/gbtree.cc:84: DANGER AHEAD: You have manually specified `updater` parameter. The `tree_method` parameter will be ignored. Incorrect sequence of updaters will produce undefined behavior. For common uses, we recommend using `tree_method` parameter instead.

⠋ |         1 | update trees in an existing model                               [15:46:35] WARNING: src/gbm/gbtree.cc:84: DANGER AHEAD: You have manually specified `updater` parameter. The `tree_method` parameter will be ignored. Incorrect sequence of updaters will produce undefined behavior. For common uses, we recommend using `tree_method` parameter instead.

⠹ |         3 | update trees in an existing model                               [15:46:35] WARNING: src/gbm/gbtree.cc:84: DANGER AHEAD: You have manually specified `updater` parameter. The `tree_method` parameter will be ignored. Incorrect sequence of updaters will produce undefined behavior. For common uses, we recommend using `tree_method` parameter instead.

⠦ |         7 | update trees in an existing model                               [15:46:35] WARNING: src/gbm/gbtree.cc:84: DANGER AHEAD: You have manually specified `updater` parameter. The `tree_method` parameter will be ignored. Incorrect sequence of updaters will produce undefined behavior. For common uses, we recommend using `tree_method` parameter instead.

⠇ |         9 | update trees in an existing model                               [15:46:35] WARNING: src/gbm/gbtree.cc:84: DANGER AHEAD: You have manually specified `updater` parameter. The `tree_method` parameter will be ignored. Incorrect sequence of updaters will produce undefined behavior. For common uses, we recommend using `tree_method` parameter instead.

⠼ |        15 | update trees in an existing model                               [15:46:35] WARNING: src/gbm/gbtree.cc:84: DANGER AHEAD: You have manually specified `updater` parameter. The `tree_method` parameter will be ignored. Incorrect sequence of updaters will produce undefined behavior. For common uses, we recommend using `tree_method` parameter instead.

⠇ |        19 | update trees in an existing model                               
✔ |        22 | update trees in an existing model [1.8s]

══ Results ═════════════════════════════════════════════════════════════════════
Duration: 180.5 s

── Skipped tests  ──────────────────────────────────────────────────────────────
• empty test (1)

[ FAIL 1 | WARN 2 | SKIP 1 | PASS 1317 ]
Error: Test failures
Execution halted

I will look into this a bit more. In most cases these are fixable. (It was quite bad initially due to R itself, but I have fixed it, now they almost never show up with R packages.)

P. S. This is without OpenMP, but it is inconsequential for the issue.

@trivialfis
Copy link
Member

Thank you for looking into this. Out of curiosity, is mac ppc still available?

@barracuda156
Copy link
Author

Thank you for looking into this. Out of curiosity, is mac ppc still available?

@trivialfis Apple obviously dropped PPC support (starting from 10.7 ppc slices are gone, ppc64 dropped with 10.6), but macOS PPC is still used – as long as hardware is available, at least.
We support PPC in Macports, so testing is normally done across all three archs (ppc, x86, aarch64).

@trivialfis
Copy link
Member

trivialfis commented Mar 20, 2023

I'm not sure if we can fix it though, or if the fix is worth the effort. None of us have access to such device, it's very likely that the next commit will just break it, especially now that xgboost is using c++-17 (CRAN is using c++-17 by default as well, AFAIK).

@barracuda156
Copy link
Author

barracuda156 commented Mar 20, 2023

I'm not sure if we can fix it though, or if the fix is worth the effort. None of us have access to such device, it's very likely that the next commit will just break it, especially now that xgboost is using c++-17 (CRAN is using c++-17 by default as well, AFAIK).

@trivialfis Oh, don’t worry about malloc issue, it is on me to fix locally for Macports. Here we just need to fix OpenMP detection with GCC (which is not specific to OS version or arch).

C++17 is fully supported on macOS 10.4+ and on ppc/ppc64. No problems here.

P. S. Could you take a quick look at this? #8947
Removing static assert seems to work fine, but better verify that.

@trivialfis
Copy link
Member

Regarding the static assert, we can change size_t to uint64_t. The former is implementation defined while the later is standardized. No worries there, we can fix it.

@barracuda156
Copy link
Author

barracuda156 commented Mar 23, 2023

@trivialfis Quick update: I fixed configure script for Macports environment, it works with GCC now, but I may need a bit of your assistance to write it neatly for a general case.
(Also I have no idea where Homebrew installs stuff and cannot test its case locally – but I guess, we want GCC to be fixed with Homebrew as well?)

Macports installs libomp into /opt/local/lib/libomp and /opt/local/include/libomp, so for a case of Macports+Clang ldflags would be -L/opt/local/lib/libomp -lomp, like here: https://github.com/macports/macports-ports/blob/79a69efbd26e8c30c8a749b357f04f99ee813af3/math/libfaiss/Portfile#L37-L43
For Macports+GCC ldflags would be -L/opt/local/lib/libgcc -lgomp (but likely -fopenmp alone gonna work as well). GCC includes are in a rather obscure path, but they are not needed, GCC is smart enough to handle that (Clang… not so much).

P. S. As you can see, linking works now:

$ otool -L /opt/local/Library/Frameworks/R.framework/Versions/4.2/Resources/library/xgboost/libs/xgboost.so
/opt/local/Library/Frameworks/R.framework/Versions/4.2/Resources/library/xgboost/libs/xgboost.so:
	xgboost.so (compatibility version 0.0.0, current version 0.0.0)
	/opt/local/lib/libMacportsLegacySupport.dylib (compatibility version 1.0.0, current version 1.0.7)
	/opt/local/lib/libgcc/libgomp.1.dylib (compatibility version 2.0.0, current version 2.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 550.44.0)
	/opt/local/lib/libgcc/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.30.0)
	/opt/local/Library/Frameworks/R.framework/Versions/4.2/Resources/lib/libR.dylib (compatibility version 4.2.0, current version 4.2.3)
	/opt/local/lib/libgcc/libgcc_s.1.1.dylib (compatibility version 1.0.0, current version 1.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.11)

@trivialfis
Copy link
Member

unfortunately, that's beyond my knowledge. I don't have a mac device and am not familiar with its environment. Maybe a quick hack using DYLD_LIBRARY_PATH just to check whether xgboost can link libomp at all?

@barracuda156
Copy link
Author

unfortunately, that's beyond my knowledge. I don't have a mac device and am not familiar with its environment. Maybe a quick hack using DYLD_LIBRARY_PATH just to check whether xgboost can link libomp at all?

I know what should be done for compiler so that OpenMP works, what I am less sure of is how to rewrite that chunk of code in configure optimally.

Let me try something after I sleep. If that works locally I need someone to confirm it also works with Homebrew as well.

@trivialfis
Copy link
Member

Thank you for working on this!

@hcho3 would be great if you can have a look when you are available. ;-)

@barracuda156
Copy link
Author

Just for the reference, here is a temporary fix for Macports: https://github.com/macports/macports-ports/tree/master/R/R-xgboost/files
And after applying patches, this: https://github.com/macports/macports-ports/blob/fb898c6c1136b54f471d0f01895859502245d346/R/R-xgboost/Portfile#L22-L38 (since Macports may be installed in a non-default prefix, we cannot just hardcode /opt/local).

This, obviously, cannot be used here, but that logic works – it just has to be adapted conditionally.

@hcho3
Copy link
Collaborator

hcho3 commented Mar 23, 2023

@barracuda156 I notice that the Portfile is patching in the prefix paths. How should we handle prefix paths in the vanilla configure script? (It can't do what Portfile does.) How about asking the user to set some environment variables?

@barracuda156
Copy link
Author

barracuda156 commented Mar 23, 2023

@barracuda156 I notice that the Portfile is patching in the prefix paths. How should we handle prefix paths in the vanilla configure script? (It can't do what Portfile does.) How about asking the user to set some environment variables?

@hcho3 I think we can use the default prefix for Macports case (/opt/local). If the current version of configure script is able to detect Homebrew (seems so) and then uses fallback if no Homebrew detected, fallback can be made more meaningful by using default Macports settings:

if test `uname -s` = "Darwin"
then
  if command -v brew &> /dev/null
  then
    OMP_PREFIX=`brew --prefix libomp`
  else
    # Macports prefix:
    OMP_PREFIX=/opt/local
  fi

So the logic will be: 1) test for Homebrew, if true, use existing configure, if false, fallback to using Macports; 2) test for compiler being used, if Clang, use Clang-specific flags, if GCC (or just else), use GCC-specific flags.
If it is desirable to have GCC build fixed with Homebrew as well, then we have 4 cases: Homebrew+Clang, Homebrew+GCC, Macports+Clang, Macports+GCC. I do not know correct flags for Homebrew installation, but Macports ones are quoted above (notice that OpenMP paths are a bit different in a case of Macports, we cannot merely change the prefix from /usr/local to /opt/local).

@hcho3
Copy link
Collaborator

hcho3 commented Mar 23, 2023

Homebrew GCC should work with -fopenmp -lomp; no special flags needed.

My plan now is to test the following scenarios in order:

  1. Homebrew + Apple Clang
  2. Homebrew + GCC
  3. Macports + Clang
  4. Macports + GCC

@barracuda156 For GCC, do we need to specify -L/opt/local/lib/libgcc, or can it be omitted safely?

@barracuda156
Copy link
Author

barracuda156 commented Mar 23, 2023

Homebrew GCC should work with -fopenmp -lomp; no special flags needed.

My plan now is to test the following scenarios in order:

  1. Homebrew + Apple Clang
  2. Homebrew + GCC
  3. Macports + Clang
  4. Macports + GCC

That will make sense. (BTW yeah, I have set Macports’s R to use LLVM Clangs on x86, so we indeed have non-Apple Clang in this case. Actually, I had an impression that Apple Clang does not support OpenMP, or at least that was the case on many Intel macOS; does it actually work with Homebrew or do they also use LLVM Clang instead?)

@barracuda156 For GCC, do we need to specify -L/opt/local/lib/libgcc, or can it be omitted safely?

Let me try throwing that out and see if it still works. Theoretically, GCC only needs -fopenmp and nothing else; in practice, it is normally the case, but I have encountered a few exceptions (perhaps due to inappropriately written configure, but w/e).

@trivialfis
Copy link
Member

trivialfis commented Aug 9, 2023

@hcho3 @jameslamb I recently learned that there's an openmp macro in autotools AC_OPENMP. I have tested it works well on Linux (Ubuntu + gcc-11.4), not sure if it can be useful for simplifying the logic.

@barracuda156
Copy link
Author

@trivialfis I guess it merely checks if OpenMP is supported by the compiler, but does not set correct flags. So to make it work, flags should still be conditional, otherwise either GCC builds will break or Clang ones.

@trivialfis
Copy link
Member

It does supply the -fopenmp flag on Linux for GCC.

If the current language is C, the macro AC_OPENMP sets the variable OPENMP_CFLAGS to the C compiler flags needed for supporting OpenMP.

@barracuda156
Copy link
Author

It does supply the -fopenmp flag on Linux for GCC.

This is needed for both compilers, but Clang will likely need -L${location_of_libomp} -lomp, while the same should not be used with GCC.

@trivialfis
Copy link
Member

Ah, thank you for sharing.

@barracuda156
Copy link
Author

@trivialfis I assume if libomp is installed in the default path, -lomp is enough (Macports installs libomp into ${prefix}/lib/libomp or smth alike, don’t ask me why, I do not deal with LLVM stuff), but this flag should be used only with Clang in any case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants