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

getDTthreads more robust #2708

merged 3 commits into from Mar 28, 2018

getDTthreads more robust #2708

merged 3 commits into from Mar 28, 2018


Copy link

@mattdowle mattdowle commented Mar 28, 2018

Attempt 1 for #2707

If user calls setDTthread(0) it was calling omp_set_num_threads() to omp_get_thread_limit() which was INT_MAX by default. Relatively new fsort was using DTthreads without restriction, whereas usage elsewhere was typically limiting number of threads based on chunk size or similar.
All usage of omp_set.* removed and the grep in CRAN_Release.cmd expanded to include openmp-utils.c, too.

@mattdowle mattdowle added this to the v1.10.6 milestone Mar 28, 2018
Copy link

@codecov-io codecov-io commented Mar 28, 2018

Codecov Report

Merging #2708 into master will decrease coverage by <.01%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2708      +/-   ##
- Coverage   93.33%   93.32%   -0.01%     
  Files          61       61              
  Lines       12220    12225       +5     
+ Hits        11405    11409       +4     
- Misses        815      816       +1
Impacted Files Coverage Δ
R/openmp-utils.R 100% <100%> (ø) ⬆️
src/openmp-utils.c 88.88% <84.61%> (-2.03%) ⬇️

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 f607162...c387fce. Read the comment docs.

@mattdowle mattdowle merged commit 437646f into master Mar 28, 2018
4 of 6 checks passed
@mattdowle mattdowle deleted the getDTthread branch Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants