Implementing unit tests directly for c++ to clarify specifications. Add more comments in the cpp code particularly around the (collated) order of the names and the nested loops.

This was fixed in is.overlapping by replacing the walrus operator (:=) with a set function in savestate (the set function returns an intelligible error message whereas the walrus operator when applied to a data.table loaded from disk may silently fail to add the column).

there are more walrus operators in the intervalaverage function that either need to be replaced with calls to the set function or else you need to ensure that memory is allocated for the data.table before proceeding with the intervalaverage function.

It would be great to write a unit test for this but it's slightly involved since you need to write a file to disk and include it with the package.

Likely traced to a bug in data.table::foverlaps reported here
Rdatatable/data.table#4580

I could switch all the code to equivalent non-equijoins but previous timings I've done suggest that foverlaps is faster than an equivalent non-equijoin. Plus, the non-equijoin doesn't retain both sets of the interval join columns so extra work would need to be done around column naming to reproduce the exact functionality of foverlaps. It's probably better to leave the code as is and wait for a fix in data.table since all intervalaverage checks are passing on windows 64 bit and linux.

It may be possible to substantially reduce the intermediate large allocation that occurs in intervalaverage::intervalaverage but this depends on some unimplemented features in data.table.

If #11 gets implemented AND #13 gets implemented AND .EACHI becomes possible for foverlaps (but grouping would need to be within groups of x=y in the intervalaverage::intervalaverage foverlaps(x=y,y=x) call), weighted-averages within groups could happen by group returning the result rather than storing the whole intermediate table (this is what EACHI does for normal [.data.table calls). GForce would probably not work for this (I'm pretty sure EACHI and GForce don't work together), so timing would need to be done to see if there's a tradeoff between memory usage and timing because not using Gforce optimization may be slower.

Rdatatable/data.table#4304

I'm not aware of any packages which have implemented this specific functionality of averaging from values recorded over specific intervals to arbitrary intervals. If I find something similar it's worth benchmarking for speed and ideally memory usage too (although this is harder).

It would be useful to create a diagram in the vignette demonstrating how the averaging works for visual learners.

NAs in interval_vars leads to this error: columns corresponding to interval_vars cannot be missing in x (or y)

What would you like to see happen?

This should be clarified to say NAs are not allowed in these columns. It is otherwise confusingly similar to the error where the specified interval_vars columns just don't exist in the input.

in is.overlapping

  if(class(start)!=class(end)){     stop("start and end must be of the same class")   }

will result in this warning

In if (class(start) != class(end)) { ... :
  the condition has length > 1 and only the first element will be used

if object has multiple classes

This bug Rdatatable/data.table#4603 necessitates setting the key of the output (q) to NULL then setting it again to the group_vars, interval_vars. Once that bug is fixed, you can increase data.table dependency to the version that fixes it and eliminate the extra setkey

May eventually be calculated as part of foverlaps, eliminating the need for pmin/pmax calls:
Rdatatable/data.table#1975

also allow different names for group_vars in intervalintersect

http://www.stats.bris.ac.uk/R/web/packages/bit64/index.html

allowing start and end values to be stored as bit64::integer would be useful in the context of representing many years of data as seconds. integer is insufficient for this purpose because it can't represent a range of more than ~40ish (?) years.

state function are used right now to restore arguments back to their original state (row order, remove additional columns) prior to function exit.

Rdatatable/data.table#4575 suggests row reordering can be greatly simplified implemented once Rdatatable/data.table#4386 is merged

Internal kaufman-lab processes use this as a criteria to determine if an average meets "completeness" requirements.

maximum gap is defined as the maximum length in a y interval of any concurrent run of time-points for which a specific x value is missing. Missingness could be either structural (ie part of a y interval isn't in x at all) or explicit (a row in x/specific x interval has NA for a value)..

add a similar check to the existing one for x

 #stop if interval starts are before interval ends
  if(x[, sum(.SD[[2]]-.SD[[1]] <0)!=0,.SDcols=interval_vars]){
    stop("there exist values in x[[interval_vars[1] ]] that are
         less than corresponding values in  x[[interval_vars[2] ]].
         interval_vars must specify columns corresponding to increasing intervals")
  }

also add unit tests for this (it should return an error)

If GForce weighted average gets implemented, the code for intervalaverage::intervalaverage could be greatly simplified.

perhaps in a vignette?
https://towardsdatascience.com/using-the-pandas-resample-function-a231144194c4

There are similarities between resample and intervalaverage,

• resample is quite a bit less flexible in that it only resamples to regular, pre-specified periods. intervalaverage resamples to arbitrary periods and they don't need to be regular (or even non-overlapping).
• resample doesn't seem to resample to different periods for different subsets. This flexibility is useful, for example, if you want to calculate a different time-average for different individuals. (for example, taking an average of air pollution in the 365 days prior to each individual's death)
• On the other hand intervalaverage requires specification of the desired periods, so it lacks the convenience of resample for those specific cases.
• Additionally, resample allows left, right, or completely open intervals. intervalaverage assumes completely closed intervals. This difference makes sense because intervalaverage was designed specifically for data measured over intervals, possibly irregular intervals. Resample is stored via a single column. Intervalaverage data requires start and end times to be stored as separate columns, so the user can add/subtract 1 to the start and ends to specify intervals which are both inclusive and correspond to the desired interval.

testing shows it's slow relative when repeated over groups relative to doing this manually using foverlaps, probably because a data.table gets created for each group and that has overhead. If .SD kept keys then you could accomplish the task via

setkey(x, g, start,end)
x[, is.overlapping(.SD),by="g"]

but right now the key of .SD is NULL rather than start, end so currently this doesn't work (returns error message saying you can't reorder .SD). I think there's an issue in data.table to keep the key of .SD in this situation which would allow that code to work.

https://devguide.ropensci.org/ci.html

intervalaverage v 0.8 and data.table 1.12.8 is faster than v 0.9 and 1.13.x

when running a large intervalaverage (mesa events analysis) . this might be due to the addition of maxgaps, or possibly data.table performance regression.

create a publicly shareable reprex and identify what's causing it.

for speed

https://stackoverflow.com/questions/66860659/lazydata-is-specified-without-a-data-directory-error-when-submitting-r-packa

https://cran.r-project.org/web/checks/check_results_intervalaverage.html

right now xminstart and xmaxend are just based on the minimum and maximum overlapping interval, whether or not the value_var for that row in x is missing.

value_var specific xminstart and xmaxend could check the earliest (and latest) date for which that value is nonmissing

Right now this does a self-join and counts number of rows which is totally unnecessary. If there are many overlapping rows the memory usage could blow up rather than just returning TRUE on the first detection of an overlap.

Implement directly in c++ using a start/stop in/out approach to find overlaps and break at the first sign if the overlap will be faster than foverlaps because foverlaps completes the join even when matches have been detected.

I had a dataset where there were some ID's in my exposure window data that didn't appear in the prediction data.

Error in setstate(x, statex):
all required_columns %in% names(x) is not TRUE
Error in setstate(y, statey):
all required_columns %in% names(x) is not TRUE

then when I checked head(y) I saw that the names of y had been changed to "intervalaverage_end_original_class_copy" and "intervalaverage_start_original_class_copy"

This needs a more informative error and ideally it wouldn't alter the data object that's being passed in.

This is second-hand from Altan. After calling intervalaverage, printing is invisible the first time y is printed (possibly x too). Standard data.table stuff where if you make an assignment by reference, the next time that table prints it will do so invisibly.

I just need to do add x[] and y[] in the on.exit call to avoid this.

or needs to be deleted if it slows things down too much

https://en.wikipedia.org/wiki/Kahan_summation_algorithm

also switch tests from expect_equal to expect_identical

and write tests that add small and large numbers