add floyd-rivest selection and parallelization to gmedian#7481
add floyd-rivest selection and parallelization to gmedian#7481ben-schwen wants to merge 6 commits intomasterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7481 +/- ##
==========================================
- Coverage 99.00% 98.98% -0.02%
==========================================
Files 87 87
Lines 16893 16879 -14
==========================================
- Hits 16725 16708 -17
- Misses 168 171 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Generated via commit 19f0b8d Download link for the artifact containing the test results: ↓ atime-results.zip
|
Co-authored-by: Michael Chirico <[email protected]>
| else subd[j-nacount] = xd[k]; | ||
| #pragma omp parallel num_threads(getDTthreads(ngrp, true)) | ||
| { | ||
| double *thread_subd = malloc(maxgrpn * sizeof(double)); |
There was a problem hiding this comment.
Does it make sense to allocate outside of parallel and use R alloc, so the memory usage can be tracked by apps that track memory usage using R API.
| if (nacount && !narm) { | ||
| ansd[i] = NA_REAL; | ||
| } else { | ||
| // Use Floyd-Rivest for groups larger than 100, quickselect for smaller |
There was a problem hiding this comment.
Does it make sense to put 100 as a parameter, so we can explore impact of this code path branches by options
There was a problem hiding this comment.
Could do. Like all of these parameters, it should be tuned.

Not directly demanded but improving benchmarks
Details