Skip to content

Remove OpenMP dependency - #300

Merged
chyunsu3 merged 12 commits into
dmlc:mainlinefrom
chyunsu3:remove_openmp
Jul 16, 2021
Merged

Remove OpenMP dependency#300
chyunsu3 merged 12 commits into
dmlc:mainlinefrom
chyunsu3:remove_openmp

Conversation

@chyunsu3

@chyunsu3 chyunsu3 commented Jul 16, 2021

Copy link
Copy Markdown
Collaborator

Follow-up to #299.

On MacOS, the latest OpenMP runtime (libomp) is causing Treelite to hang. Fortunately, it is relatively painless to remove the use of OpenMP from Treelite, as only one feature of Treelite (branch annotator) uses it. (The Treelite runtime uses a custom thread pool.)

Replace OpenMP with C++11 threads.

As a side effect, this PR also speeds up testing on MacOS, because we no longer have to install packages from Homebrew. This shaves off 15 min of CI time.

@codecov

codecov Bot commented Jul 16, 2021

Copy link
Copy Markdown

Codecov Report

Merging #300 (f206ffe) into mainline (59217af) will increase coverage by 0.20%.
The diff coverage is 97.77%.

Impacted file tree graph

@@              Coverage Diff               @@
##             mainline     #300      +/-   ##
==============================================
+ Coverage       84.98%   85.18%   +0.20%     
  Complexity         46       46              
==============================================
  Files             106      108       +2     
  Lines            8040     8156     +116     
  Branches          461       50     -411     
==============================================
+ Hits             6833     6948     +115     
- Misses           1182     1183       +1     
  Partials           25       25              
Impacted Files Coverage Δ
tests/cpp/test_threading_utils.cc 96.34% <96.34%> (ø)
src/annotator.cc 92.26% <100.00%> (+0.78%) ⬆️
src/threading_utils/parallel_for.h 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59217af...f206ffe. Read the comment docs.

Comment thread src/CMakeLists.txt
Comment thread src/threading_utils/parallel_for.h Outdated
Comment thread src/threading_utils/parallel_for.h Outdated

@trivialfis trivialfis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@chyunsu3
chyunsu3 merged commit 45b803f into dmlc:mainline Jul 16, 2021
@chyunsu3
chyunsu3 deleted the remove_openmp branch July 16, 2021 09:36
This was referenced Jul 16, 2021
rapids-bot Bot pushed a commit to NVIDIA/cuml that referenced this pull request Jul 21, 2021
Closes #4037.

The 2.0.0 version of Treelite incorporates the following major improvements:

* **Remove dmlc-core dependency** (dmlc/treelite#284).
* Rename CHECK and LOG macros to avoid name collisions (dmlc/treelite#295)
* Suppress warning about build time when parallel_comp is specified (dmlc/treelite#297)
* Handle LightGBM models with '=' in feature names (dmlc/treelite#298)
* Remove OpenMP dependency (dmlc/treelite#300)

Depends on rapidsai/integration#315

Authors:
  - Philip Hyunsu Cho (https://github.com/hcho3)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Jordan Jacobelli (https://github.com/Ethyling)

URL: #4072
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
Closes NVIDIA#4037.

The 2.0.0 version of Treelite incorporates the following major improvements:

* **Remove dmlc-core dependency** (dmlc/treelite#284).
* Rename CHECK and LOG macros to avoid name collisions (dmlc/treelite#295)
* Suppress warning about build time when parallel_comp is specified (dmlc/treelite#297)
* Handle LightGBM models with '=' in feature names (dmlc/treelite#298)
* Remove OpenMP dependency (dmlc/treelite#300)

Depends on rapidsai/integration#315

Authors:
  - Philip Hyunsu Cho (https://github.com/hcho3)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Jordan Jacobelli (https://github.com/Ethyling)

URL: NVIDIA#4072
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

Successfully merging this pull request may close these issues.

2 participants