frollmax PR as potentially complete

Author: jangorecki

R/froll.R

@@ -37,7 +37,7 @@ partial2adaptive = function(x, n, align, adaptive) {
     if (!is.numeric(n))
       stopf("n must be an integer vector or a list of integer vectors")
     if (verbose)
-      cat("partial2adaptive: froll partial=TRUE trimming 'n' and redirecting to adaptive=TRUE\n")
+      catf("partial2adaptive: froll partial=TRUE trimming 'n' and redirecting to adaptive=TRUE\n")
     if (length(n)>1L) {
       lapply(n, len, align, FUN=trimn)
     } else {
@@ -53,7 +53,7 @@ partial2adaptive = function(x, n, align, adaptive) {
     if (length(n[[1L]]) != len)
       stopf("length of vectors in 'x' must match to length of adaptive window in 'n'")
     if (verbose)
-      cat("partial2adaptive: froll adaptive=TRUE and partial=TRUE trimming 'n'\n")
+      catf("partial2adaptive: froll adaptive=TRUE and partial=TRUE trimming 'n'\n")
     lapply(n, align, FUN=trimnadaptive)
   }
 }
@@ -73,7 +73,7 @@ froll = function(fun, x, n, fill=NA, algo, align=c("right","left","center"), na.
     verbose = getOption("datatable.verbose")
     rev2 = function(x) if (is.list(x)) lapply(x, rev) else rev(x)
     if (verbose)
-      cat("froll: adaptive=TRUE && align='left' pre-processing for align='right'\n")
+      catf("froll: adaptive=TRUE && align='left' pre-processing for align='right'\n")
     x = rev2(x)
     n = rev2(n)
     align = "right"
@@ -84,7 +84,7 @@ froll = function(fun, x, n, fill=NA, algo, align=c("right","left","center"), na.
     ans = .Call(CfrollapplyR, FUN, x, n, fill, align, adaptive, rho)
   if (leftadaptive) {
     if (verbose)
-      cat("froll: adaptive=TRUE && align='left' post-processing from align='right'\n")
+      catf("froll: adaptive=TRUE && align='left' post-processing from align='right'\n")
     ans = rev2(ans)
   }
   if (isTRUE(give.names) && is.list(ans)) {
@@ -122,8 +122,6 @@ frollmax = function(x, n, fill=NA, algo=c("fast","exact"), align=c("right","left
 }
 
 frollapply = function(x, n, FUN, ..., fill=NA, align=c("right","left","center"), adaptive=FALSE, partial=FALSE, give.names=FALSE) {
-  if (isTRUE(adaptive) && base::getRversion() < "3.4.0") ## support SET_GROWABLE_BIT
-    stopf("frollapply adaptive=TRUE requires at least R 3.4.0"); # nocov
   FUN = match.fun(FUN)
   rho = new.env()
   froll(FUN=FUN, rho=rho, x=x, n=n, fill=fill, align=align, adaptive=adaptive, partial=partial, give.names=give.names)

man/froll.Rd

@@ -126,7 +126,6 @@
 }
 \section{\code{frollapply}}{
   \code{frollapply} computes rolling aggregate on arbitrary R functions. \code{adaptive} argument is not supported (to be changed). The input \code{x} (first argument) to the function \code{FUN} is coerced to \emph{numeric} beforehand(to be changed) and \code{FUN} has to return a scalar \emph{numeric} value (to be changed). Checks for that are made only during the first iteration when \code{FUN} is evaluated. Edge cases can be found in examples below. Any R function is supported, but it is not optimized using our own C implementation -- hence, for example, using \code{frollapply} to compute a rolling average is inefficient. It is also always single-threaded because there is no thread-safe API to R's C \code{eval}. Nevertheless we've seen the computation speed up vis-a-vis versions implemented in base R.
-  Support for \code{adaptive=TRUE} in \code{frollapply} requires R 3.4.0 or higher.
 }
 \section{\code{zoo} package users notice}{
   Users coming from most popular package for rolling functions \code{zoo} might expect following differences in \code{data.table} implementation

src/data.table.h

@@ -16,9 +16,6 @@
 #if R_VERSION < R_Version(4, 5, 0)
 #  define isDataFrame(x) isFrame(x) // #6180
 #endif
-#if !defined(R_VERSION) || R_VERSION < R_Version(3, 4, 0)
-#  define SET_GROWABLE_BIT(x) // frollapply adaptive #5441
-#endif
 #include <Rinternals.h>
 #define SEXPPTR_RO(x) ((const SEXP *)DATAPTR_RO(x))  // to avoid overhead of looped STRING_ELT and VECTOR_ELT
 #include <stdint.h>    // for uint64_t rather than unsigned long long

src/froll.c

@@ -573,10 +573,9 @@ void frollmaxExact(double *x, uint64_t nx, ans_t *ans, int k, double fill, bool
       ans->dbl_v[i] = w;
     }
   } else {
-    bool *isnan = malloc(nx*sizeof(bool));                          // isnan lookup - we use it to reduce ISNAN calls in nested loop
+    bool *isnan = malloc(nx*sizeof(*isnan));                        // isnan lookup - we use it to reduce ISNAN calls in nested loop
     if (!isnan) {                                                   // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for isnan", __func__); // raise error
-      free(isnan);
       return;
     }                                                               // # nocov end
     bool truehasnf = hasnf>0;

src/frolladaptive.c

@@ -53,7 +53,6 @@ void frolladaptivemeanFast(double *x, uint64_t nx, ans_t *ans, int *k, double fi
   double *cs = malloc(sizeof(*cs) * nx);                       // cumsum vector, same as double cs[nx] but no segfault
   if (!cs) {                                                    // # nocov start
     ansSetMsg(ans, 3, "%s: Unable to allocate memory for cumsum", __func__); // raise error
-    free(cs);
     return;
   }                                                             // # nocov end
   if (!truehasnf) {
@@ -82,22 +81,22 @@ void frolladaptivemeanFast(double *x, uint64_t nx, ans_t *ans, int *k, double fi
   }
   if (truehasnf) {
     uint64_t nc = 0, pinf = 0, ninf = 0;                        // running NA counter
-    uint64_t *cn = malloc(nx*sizeof(uint64_t));                 // cumulative NA counter, used the same way as cumsum, same as uint64_t cn[nx] but no segfault
+    uint64_t *cn = malloc(nx*sizeof(*cn));                      // cumulative NA counter, used the same way as cumsum, same as uint64_t cn[nx] but no segfault
     if (!cn) {                                                  // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for cum NA counter", __func__); // raise error
-      free(cs); free(cn);
+      free(cs);
       return;
     }                                                           // # nocov end
-    uint64_t *cpinf = malloc(nx*sizeof(uint64_t));
+    uint64_t *cpinf = malloc(nx*sizeof(*cpinf));
     if (!cpinf) {                                               // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for cum Inf counter", __func__); // raise error
-      free(cs); free(cn); free(cpinf);
+      free(cs); free(cn);
       return;
     }                                                           // # nocov end
-    uint64_t *cninf = malloc(nx*sizeof(uint64_t));
+    uint64_t *cninf = malloc(nx*sizeof(*cninf));
     if (!cninf) {                                               // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for cum -Inf counter", __func__); // raise error
-      free(cs); free(cn); free(cpinf); free(cninf);
+      free(cs); free(cn); free(cpinf);
       return;
     }                                           // # nocov end
     for (uint64_t i=0; i<nx; i++) {                             // loop over observations to calculate cumsum and cum NA counter
@@ -277,7 +276,6 @@ void frolladaptivesumFast(double *x, uint64_t nx, ans_t *ans, int *k, double fil
   double *cs = malloc(sizeof(*cs) * nx);
   if (!cs) {                                                    // # nocov start
     ansSetMsg(ans, 3, "%s: Unable to allocate memory for cumsum", __func__); // raise error
-    free(cs);
     return;
   }                                                             // # nocov end
   if (!truehasnf) {
@@ -306,22 +304,22 @@ void frolladaptivesumFast(double *x, uint64_t nx, ans_t *ans, int *k, double fil
   }
   if (truehasnf) {
     uint64_t nc = 0, pinf = 0, ninf = 0;                        // running NA counter
-    uint64_t *cn = malloc(nx*sizeof(uint64_t));                 // cumulative NA counter, used the same way as cumsum, same as uint64_t cn[nx] but no segfault
+    uint64_t *cn = malloc(nx*sizeof(*cn));                 // cumulative NA counter, used the same way as cumsum, same as uint64_t cn[nx] but no segfault
     if (!cn) {                                                  // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for cum NA counter", __func__); // raise error
-      free(cs); free(cn);
+      free(cs);
       return;
     }                                                           // # nocov end
-    uint64_t *cpinf = malloc(nx*sizeof(uint64_t));
+    uint64_t *cpinf = malloc(nx*sizeof(*cpinf));
     if (!cpinf) {                                               // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for cum Inf counter", __func__); // raise error
-      free(cs); free(cn); free(cpinf);
+      free(cs); free(cn);
       return;
     }                                                           // # nocov end
-    uint64_t *cninf = malloc(nx*sizeof(uint64_t));
+    uint64_t *cninf = malloc(nx*sizeof(*cninf));
     if (!cninf) {                                               // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for cum -Inf counter", __func__); // raise error
-      free(cs); free(cn); free(cpinf); free(cninf);
+      free(cs); free(cn); free(cpinf);
       return;
     }                                                           // # nocov end
     for (uint64_t i=0; i<nx; i++) {                             // loop over observations to calculate cumsum and cum NA counter
@@ -492,10 +490,9 @@ void frolladaptivemaxExact(double *x, uint64_t nx, ans_t *ans, int *k, double fi
       }
     }
   } else {
-    bool *isnan = malloc(nx*sizeof(bool)); // isnan lookup - we use it to reduce ISNAN calls in nested loop
+    bool *isnan = malloc(nx*sizeof(*isnan)); // isnan lookup - we use it to reduce ISNAN calls in nested loop
     if (!isnan) {                                                    // # nocov start
       ansSetMsg(ans, 3, "%s: Unable to allocate memory for isnan", __func__); // raise error
-      free(isnan);
       return;
     }                                                               // # nocov end
     bool truehasnf = hasnf>0;