Merge branch 'master' into modular_gforce

Author: ben-schwen

NEWS.md

@@ -18,6 +18,14 @@
 
 2. `[,showProgress=]` and `options(datatable.showProgress)` now accept an integer to control the progress bar update interval in seconds, allowing finer control over progress reporting frequency; `TRUE` uses the default 3-second interval, [#6514](https://github.com/Rdatatable/data.table/issues/6514). Thanks @ethanbsmith for the report and @ben-schwen for the PR.
 
+3. GForce and lapply optimization detection has been refactored to use modular optimization paths and an AST (Abstract Syntax Tree) walker for improved maintainability and extensibility. The new architecture separates optimization detection into distinct, composable phases. This makes future optimization enhancements a lot easier. Thanks to @grantmcdermott, @jangorecki, @MichaelChirico, and @HughParsonage for the suggestions and @ben-schwen for the implementation.
+
+    This rewrite also introduces several new optimizations:
+      - Enables Map in addition to lapply optimizations (e.g., `Map(fun, .SD)` -> `list(fun(col1), fun(col2), ...)`) [#5336](https://github.com/Rdatatable/data.table/issues/5336)
+      - lapply optimization works without .SD (e.g., `lapply(list(col1, col2), fun)` -> `list(fun(col1), fun(col2))` [#5032](https://github.com/Rdatatable/data.table/issues/5032)
+      - Type conversion support in GForce expressions (e.g., `sum(as.numeric(x))` will use GForce, saving the need to coerce `x` in a setup step) [#2934](https://github.com/Rdatatable/data.table/issues/2934)
+      - Arithmetic operation support in GForce (e.g., `max(x) - min(x)` will use GForce on both `max(x)` and `min(x)`, saving the need to do the subtraction in a follow-up step) [#3815](https://github.com/Rdatatable/data.table/issues/3815)
+
 ### BUG FIXES
 
 1. `fread()` with `skip=0` and `(header=TRUE|FALSE)` no longer skips the first row when it has fewer fields than subsequent rows, [#7463](https://github.com/Rdatatable/data.table/issues/7463). Thanks @emayerhofer for the report and @ben-schwen for the fix.
@@ -34,13 +42,7 @@
 
 7. Fixed compilation failure like "error: unknown type name 'siginfo_t'" in v1.18.0 in some strict environments, e.g., FreeBSD, where the header file declaring the POSIX function `waitid` does not transitively include the header file defining the `siginfo_t` type, [#7516](https://github.com/rdatatable/data.table/issues/7516). Thanks to @jszhao for the report and @aitap for the fix.
 
-8. GForce and lapply optimization detection has been refactored to use modular optimization paths and an AST (Abstract Syntax Tree) walker for improved maintainability and extensibility. The new architecture separates optimization detection into distinct, composable phases. This makes future optimization enhancements a lot easier. Thanks to @grantmcdermott, @jangorecki, @MichaelChirico, and @HughParsonage for the suggestions and @ben-schwen for the implementation.
-
-    This rewrite also introduces several new optimizations:
-      - Enables Map in addition to lapply optimizations (e.g., `Map(fun, .SD)` -> `list(fun(col1), fun(col2), ...)`) [#5336](https://github.com/Rdatatable/data.table/issues/5336)
-      - lapply optimization works without .SD (e.g., `lapply(list(col1, col2), fun)` -> `list(fun(col1), fun(col2))` [#5032](https://github.com/Rdatatable/data.table/issues/5032)
-      - Type conversion support in GForce expressions (e.g., `sum(as.numeric(x))` will use GForce, saving the need to coerce `x` in a setup step) [#2934](https://github.com/Rdatatable/data.table/issues/2934)
-      - Arithmetic operation support in GForce (e.g., `max(x) - min(x)` will use GForce on both `max(x)` and `min(x)`, saving the need to do the subtraction in a follow-up step) [#3815](https://github.com/Rdatatable/data.table/issues/3815)
+8. When fixing duplicate factor levels, `setattr()` no longer crashes upon encountering missing factor values, [#7595](https://github.com/Rdatatable/data.table/issues/7595). Thanks to @sindribaldur for the report and @aitap for the fix.
 
 ### Notes
 

inst/tests/tests.Rraw

@@ -21432,51 +21432,55 @@ test(2360.4, rowwiseDT(x =, expr =, 1, quote(a + b)),
 test(2360.5, rowwiseDT(x =, plist =, 1, as.pairlist(list(123))),
   error = "Column 'plist' is type 'pairlist'. Non-atomic, non-list objects must be wrapped in list\\(\\)")
 
+# setattr() must not crash for out-of-bounds factor indices when fixing duplicate levels, #7595
+test(2361.1, setattr(factor(c(1, NA), levels = 1), "levels", c("1", "1")), factor(c(1, NA)))
+test(2361.2, setattr(structure(c(-999L, 999L), class = "factor", levels = "a"), "levels", c("b", "b")), factor(c(NA, NA), levels = "b"))
+
 # gforce should also work with Map in j #5336
 # conversions should not turn gforce off #2934
 # lapply gforce should also work without .SD #5032
 # support arithmetic in j with gforce #3815
 out = c("GForce FALSE", "GForce FALSE", "GForce TRUE")
 # unwrap type conversions
 dt = data.table(a=1:4, b=1:2)
-test(2361.01, optimize=0:2, dt[, max(as.character(a)), by=b, verbose=TRUE], data.table(b=1:2, V1=c("3","4")), output=out)
-test(2361.02, optimize=0:2, dt[, max(as.numeric(a)), by=b, verbose=TRUE], data.table(b=1:2, V1=c(3,4)), output=out)
-test(2361.03, optimize=0:2, dt[, max(as.integer(as.integer(as.integer(a)))), by=b, verbose=TRUE], data.table(b=1:2, V1=3:4), output=out)
+test(2362.01, optimize=0:2, dt[, max(as.character(a)), by=b, verbose=TRUE], data.table(b=1:2, V1=c("3","4")), output=out)
+test(2362.02, optimize=0:2, dt[, max(as.numeric(a)), by=b, verbose=TRUE], data.table(b=1:2, V1=c(3,4)), output=out)
+test(2362.03, optimize=0:2, dt[, max(as.integer(as.integer(as.integer(a)))), by=b, verbose=TRUE], data.table(b=1:2, V1=3:4), output=out)
 # Map in j
 dt = data.table(a=1:4, b=1:2)
-test(2361.11, optimize=0:2, dt[, Map(sum, .SD), b, verbose=TRUE], dt[, lapply(.SD, sum), b], output=out)
-test(2361.12, optimize=0:2, dt[, Map(sum, .SD, .SD), by=b, verbose=TRUE], output="GForce FALSE")
+test(2362.11, optimize=0:2, dt[, Map(sum, .SD), b, verbose=TRUE], dt[, lapply(.SD, sum), b], output=out)
+test(2362.12, optimize=0:2, dt[, Map(sum, .SD, .SD), by=b, verbose=TRUE], output="GForce FALSE")
 # lapply without .SD
 dt = data.table(a = NA_integer_, b = 1:2, c = c(TRUE, FALSE))
-test(2361.13, optimize=0:2, dt[, Map(weighted.mean, .SD, na.rm=c), b, .SDcols="a", verbose=TRUE], data.table(b=1:2, a=c(NaN, NA_real_)), output="GForce FALSE")
-test(2361.14, optimize=0:2, dt[,list(weighted.mean(a, na.rm=c)), b, verbose=TRUE], data.table(b=1:2, V1=c(NaN, NA_real_)), output="GForce FALSE")
-test(2361.15, optimize=0:2, dt[, Map(sum, .SD), by=b, .SDcols=c("a","c"), verbose=TRUE], dt[, lapply(.SD, sum), by=b, .SDcols=c("a","c")], output=out)
+test(2362.13, optimize=0:2, dt[, Map(weighted.mean, .SD, na.rm=c), b, .SDcols="a", verbose=TRUE], data.table(b=1:2, a=c(NaN, NA_real_)), output="GForce FALSE")
+test(2362.14, optimize=0:2, dt[,list(weighted.mean(a, na.rm=c)), b, verbose=TRUE], data.table(b=1:2, V1=c(NaN, NA_real_)), output="GForce FALSE")
+test(2362.15, optimize=0:2, dt[, Map(sum, .SD), by=b, .SDcols=c("a","c"), verbose=TRUE], dt[, lapply(.SD, sum), by=b, .SDcols=c("a","c")], output=out)
 dt = data.table(a=1:2, b=1, c=1:4)
-test(2361.21, optimize=0:2, dt[, lapply(list(b, c), sum), by=a, verbose=TRUE], output=out)
-test(2361.22, optimize=0:2, dt[, c(list(sum(b), sum(c))), by=a, verbose=TRUE], output=out)
+test(2362.21, optimize=0:2, dt[, lapply(list(b, c), sum), by=a, verbose=TRUE], output=out)
+test(2362.22, optimize=0:2, dt[, c(list(sum(b), sum(c))), by=a, verbose=TRUE], output=out)
 # support arithmetic in j
 dt = data.table(a=1:4, b=1:2)
-test(2361.31, optimize=0:2, dt[, .(max(a)-min(a)), by=b, verbose=TRUE], output=out)
-test(2361.32, optimize=0:2, dt[, .((max(a) - min(a)) / (max(a) + min(a))), by=b, verbose=TRUE], data.table(b=1:2, V1=c(0.5, 1/3)), output=out)
-test(2361.33, optimize=0:2, dt[, sum(a) / .N, b, verbose=TRUE], output=out)
-test(2361.34, optimize=0:2, dt[, mean(a) * 2L + sum(a), b, verbose=TRUE], output=out)
-test(2361.35, optimize=0:2, dt[, list(range=max(a)-min(a), avg=mean(a)), by=b, verbose=TRUE], output=out)
-test(2361.36, optimize=0:2, dt[, .(max(a)-sqrt(min(a))), by=b, verbose=TRUE], output="GForce FALSE")
-test(2361.37, optimize=0:2, dt[, sum(a) %% 2, b, verbose=TRUE], output=out)
-test(2361.38, optimize=0:2, dt[, sum(a) %/% 2, b, verbose=TRUE], output=out)
-test(2361.39, optimize=0:2, dt[, -sum(a), b, verbose=TRUE], output=out)
-test(2361.40, optimize=0:2, dt[, .(sum(a)-sum(b)), b, verbose=TRUE], output="GForce FALSE")
+test(2362.31, optimize=0:2, dt[, .(max(a)-min(a)), by=b, verbose=TRUE], output=out)
+test(2362.32, optimize=0:2, dt[, .((max(a) - min(a)) / (max(a) + min(a))), by=b, verbose=TRUE], data.table(b=1:2, V1=c(0.5, 1/3)), output=out)
+test(2362.33, optimize=0:2, dt[, sum(a) / .N, b, verbose=TRUE], output=out)
+test(2362.34, optimize=0:2, dt[, mean(a) * 2L + sum(a), b, verbose=TRUE], output=out)
+test(2362.35, optimize=0:2, dt[, list(range=max(a)-min(a), avg=mean(a)), by=b, verbose=TRUE], output=out)
+test(2362.36, optimize=0:2, dt[, .(max(a)-sqrt(min(a))), by=b, verbose=TRUE], output="GForce FALSE")
+test(2362.37, optimize=0:2, dt[, sum(a) %% 2, b, verbose=TRUE], output=out)
+test(2362.38, optimize=0:2, dt[, sum(a) %/% 2, b, verbose=TRUE], output=out)
+test(2362.39, optimize=0:2, dt[, -sum(a), b, verbose=TRUE], output=out)
+test(2362.40, optimize=0:2, dt[, .(sum(a)-sum(b)), b, verbose=TRUE], output="GForce FALSE")
 # mix cases of the above
 dt = data.table(a=1:4, b=1:2)
-test(2361.41, optimize=0:2, dt[, sum(as.numeric(a)) + mean(as.integer(a)), by=b, verbose=TRUE], output=out)
-test(2361.42, optimize=0:2, dt[, Map(sum, .SD), by=b, .SDcols="a", verbose=TRUE], output=out)
-test(2361.43, optimize=0:2, dt[, lapply(list(as.numeric(a)), sum), by=b, verbose=TRUE], output="GForce FALSE")
-test(2361.44, optimize=0:2, dt[, sum(a) + as.numeric("5"), by=b, verbose=TRUE], data.table(b=1:2, V1=c(9, 11)), output="GForce FALSE")
-test(2361.45, optimize=0:2, dt[, sum(a[a > 2]), by=b, verbose=TRUE], output="GForce FALSE")
+test(2362.41, optimize=0:2, dt[, sum(as.numeric(a)) + mean(as.integer(a)), by=b, verbose=TRUE], output=out)
+test(2362.42, optimize=0:2, dt[, Map(sum, .SD), by=b, .SDcols="a", verbose=TRUE], output=out)
+test(2362.43, optimize=0:2, dt[, lapply(list(as.numeric(a)), sum), by=b, verbose=TRUE], output="GForce FALSE")
+test(2362.44, optimize=0:2, dt[, sum(a) + as.numeric("5"), by=b, verbose=TRUE], data.table(b=1:2, V1=c(9, 11)), output="GForce FALSE")
+test(2362.45, optimize=0:2, dt[, sum(a[a > 2]), by=b, verbose=TRUE], output="GForce FALSE")
 dt = data.table(a=1:4, b=1:2, c=2:5)
-test(2361.46, optimize=0:2, dt[, .(sum(a) + sum(c)), by=b, verbose=TRUE], output=out)
+test(2362.46, optimize=0:2, dt[, .(sum(a) + sum(c)), by=b, verbose=TRUE], output=out)
 # coverage and edge cases for lapply(.SD, ...)
 dt = data.table(a=1:4, b=1:2)
-test(2361.51, optimize=0:2, dt[, c(list()), b, verbose=TRUE], data.table(b=integer(0L)), output="GForce FALSE")
-test(2361.52, optimize=0:2, dt[, c(lapply(.SD, sum), list()), b, verbose=TRUE], output=out)
-test(2361.53, optimize=0:2, dt[, list(lapply(.SD, sum), list()), b, verbose=TRUE], output="GForce FALSE")
+test(2362.51, optimize=0:2, dt[, c(list()), b, verbose=TRUE], data.table(b=integer(0L)), output="GForce FALSE")
+test(2362.52, optimize=0:2, dt[, c(lapply(.SD, sum), list()), b, verbose=TRUE], output=out)
+test(2362.53, optimize=0:2, dt[, list(lapply(.SD, sum), list()), b, verbose=TRUE], output="GForce FALSE")

src/froll.c

@@ -1707,11 +1707,13 @@ void frollmedianFast(const double *x, uint64_t nx, ans_t *ans, int k, double fil
           snprintf(end(ans->message[3]), 500, _("%s: 's[A] + s[B] == h' is not true\n"), "frollmedianFast");
           return;
         }*/
-        if (n[A]!=tail && m[A] == n[A]) {
-          n[A] = tail;
-        }
-        if (n[B]!=tail && m[B] == n[B]) {
-          n[B] = tail;
+        if (even) {
+          if (n[A]!=tail && m[A] == n[A]) {
+            n[A] = tail;
+          }
+          if (n[B]!=tail && m[B] == n[B]) {
+            n[B] = tail;
+          }
         }
         ansv[j*k+i] = even ? MED2(A, B) : MED(A, B);
       }

src/wrappers.c

@@ -44,8 +44,11 @@ SEXP setlevels(SEXP x, SEXP levels, SEXP ulevels) {
   SEXP xchar, newx;
   xchar = PROTECT(allocVector(STRSXP, nx));
   int *ix = INTEGER(x);
-  for (int i=0; i<nx; ++i)
-    SET_STRING_ELT(xchar, i, STRING_ELT(levels, ix[i]-1));
+  const int nlevels = length(levels);
+  for (int i=0; i<nx; ++i) {
+    const int ixi = ix[i];
+    SET_STRING_ELT(xchar, i, (ixi >= 1 && ixi <= nlevels) ? STRING_ELT(levels, ix[i]-1) : NA_STRING);
+  }
   newx = PROTECT(chmatch(xchar, ulevels, NA_INTEGER));
   int *inewx = INTEGER(newx);
   for (int i=0; i<nx; ++i) ix[i] = inewx[i];