Skip to content

Commit

Permalink
news item shortened, and multithreaded
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdowle committed Aug 2, 2019
1 parent 5cae5c4 commit 93cc9ab
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 19 deletions.
24 changes: 8 additions & 16 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,24 +135,16 @@
20. `setkey`, `[key]by=` and `on=` in verbose mode (`options(datatable.verbose=TRUE)`) now detect any columns inheriting from `Date` which are stored as 8 byte double, test if any fractions are present, and if not suggest using a 4 byte integer instead (such as `data.table::IDate`) to save space and time, [#1738](https://github.com/Rdatatable/data.table/issues/1738). In future this could be upgraded to `message` or `warning` depending on feedback.
21. New function `fifelse(test,yes, no)` has been implemented in C by Morgan Jacob, [#3657](https://github.com/Rdatatable/data.table/issues/3657). It is comparable to `base::ifelse`, `dplyr::if_else`, `hutils::if_else`, and `vcts::if_else()`. It returns a vector of the same length as `test` but unlike `base::ifelse` the output type is consistent with those of `yes` and `no`. Please see `?data.table::fifelse` for more details.
21. New function `fifelse(test,yes, no)` has been implemented in C by Morgan Jacob, [#3657](https://github.com/Rdatatable/data.table/issues/3657). It is comparable to `base::ifelse`, `dplyr::if_else`, `hutils::if_else`, and (forthcoming) [`vctrs::if_else()`](https://vctrs.r-lib.org/articles/stability.html#ifelse). It returns a vector of the same length as `test` but unlike `base::ifelse` the output type is consistent with those of `yes` and `no`. Please see `?data.table::fifelse` for more details.

This comment has been minimized.

Copy link
@MichaelChirico

MichaelChirico Aug 2, 2019

Member

if we mentioned vctrs shouldn't we benchmark it? or it's really not implemented yet (sorry on phone)

```R
set.seed(123)
x = sample(c(TRUE,FALSE),2*5e4+1,replace = TRUE)
microbenchmark::microbenchmark(
data.table::fifelse(x, 1L, 0L),
hutils::if_else(x, 1L, 0L),
base::ifelse(x, 1L, 0L),
dplyr::if_else(x, 1L, 0L),
times = 100L
)
# Unit: microseconds
# expr min lq mean median uq max neval
# data.table::fifelse(x, 1L, 0L) 600.819 649.1415 716.5613 669.026 771.4435 1078.481 100
# hutils::if_else(x, 1L, 0L) 1053.678 1090.6680 1512.2376 1143.053 1346.1765 13827.810 100
# base::ifelse(x, 1L, 0L) 3225.178 3302.3650 4229.8110 3504.419 4230.9615 12712.126 100
# dplyr::if_else(x, 1L, 0L) 3306.855 3390.2430 4496.2153 3557.660 4466.7985 16077.566 100
# default 4 threads on a laptop with 16GB RAM and 8 logical CPU
x = sample(c(TRUE,FALSE), 5e8, replace=TRUE) # 2GB
# seconds
base::ifelse(x, 7L, 11L) # 14.3
dplyr::if_else(x, 7L, 11L) # 15.4
hutils::if_else(x, 7L, 11L) # 4.3
data.table::fifelse(x, 7L, 11L) # 0.7
```
#### BUG FIXES
Expand Down
10 changes: 7 additions & 3 deletions src/fifelse.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b)
ta = VECSXP;
}
} else {
error("'yes' is of type %s but 'no' is of type %s. Please make sure candidate replacements are of the same type.", type2char(ta),type2char(tb));
error("'yes' is of type %s but 'no' is of type %s. Please make sure candidate replacements are of the same type.", type2char(ta), type2char(tb));
}
}

Expand All @@ -51,8 +51,8 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b)
}

// Check here the length of the different input variables.
if (len1!=1 && len1!=len0) error("Length of 'yes' is %d but must be 1 or length of 'test' (%d).", len1, len0);
if (len2!=1 && len2!=len0) error("Length of 'no' is %d but must be 1 or length of 'test' (%d).", len2, len0);
if (len1!=1 && len1!=len0) error("Length of 'yes' is %lld but must be 1 or length of 'test' (%lld).", len1, len0);
if (len2!=1 && len2!=len0) error("Length of 'no' is %lld but must be 1 or length of 'test' (%lld).", len2, len0);
const int64_t amask = len1>1 ? INT64_MAX : 0;
const int64_t bmask = len2>1 ? INT64_MAX : 0;

Expand All @@ -65,6 +65,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b)
int *pans = LOGICAL(ans);
int *pa = LOGICAL(a);
int *pb = LOGICAL(b);
#pragma omp parallel for num_threads(getDTthreads())

This comment has been minimized.

Copy link
@MichaelChirico

MichaelChirico Aug 2, 2019

Member

I though the *restrict part was required for parallel loops?

for (int64_t i=0; i<len0; ++i) {
pans[i] = pl[i]==0 ? pb[i & bmask] : (pl[i]==1 ? pa[i & amask] : NA_LOGICAL);
}
Expand All @@ -73,6 +74,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b)
int *pans = INTEGER(ans);
int *pa = INTEGER(a);
int *pb = INTEGER(b);
#pragma omp parallel for num_threads(getDTthreads())
for (int64_t i=0; i<len0; ++i) {
pans[i] = pl[i]==0 ? pb[i & bmask] : (pl[i]==1 ? pa[i & amask] : NA_INTEGER);
}
Expand All @@ -81,6 +83,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b)
double *pans = REAL(ans);
double *pa = REAL(a);
double *pb = REAL(b);
#pragma omp parallel for num_threads(getDTthreads())
for (int64_t i=0; i<len0; ++i) {
pans[i] = pl[i]==0 ? pb[i & bmask] : (pl[i]==1 ? pa[i & amask] : NA_REAL);
}
Expand All @@ -96,6 +99,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b)
Rcomplex *pans = COMPLEX(ans);
Rcomplex *pa = COMPLEX(a);
Rcomplex *pb = COMPLEX(b);
#pragma omp parallel for num_threads(getDTthreads())
for (int64_t i=0; i<len0; ++i) {
pans[i] = pl[i]==0 ? pb[i & bmask] : (pl[i]==1 ? pa[i & amask] : NA_CPLX);
}
Expand Down

0 comments on commit 93cc9ab

Please sign in to comment.