Prediction cards: Memory and runtime optimization

[tildechris]

I ran a mem+cpu profile of create_prediction_cards using profvis to investigate high memory use and long runtimes when building the prediction cards.

profiler_run.zip

You should be able to open the profile_run in RStudio, or just open it in a browser (it's html) after unzipping. Beware because of the long run-time, it's huge.

Findings

There are three main sections:

1. get_covidhub_predictions: Which is downloading the predictions from github and loading/processing the CSVs into memory
2. Filtering the combined file, most of which is spent in:

# Only accept forecasts made Monday or earlier
  predictions_cards = predictions_cards %>%
                        filter(target_end_date - (forecast_date + 7 * ahead) >= -2)

3. Saving the RDS file

In get_covidhub_predictions there are many blocks of processing for each forecaster/epiweek, which are split into two parts

1. An HTTP call to grab the data
2. CSV processing

Recommendation

1. Create a pipline that processes data on a per-forecaster basis, instead of batching operations for all forecasters.
2. Parallelize processing multiple forecasters at once
3. Merge them as a last step into one predictions_cards file and save.
4. Evaluate filter operations to see which filters can't be applied sooner, perhaps when loading the CSVs.

(1) should present a more constant memory usage (instead of linear WRT # of forecasters currently). GC should be good at reclaiming memory after a particular forecaster is complete. Because of the amount of I/O in (1), the task should be able to take advantage of multi-processing. The only consideration I can think of is how to break the rbind of old and new cards on a per-forecaster basis.

Merging and saving the final result will still take linear time with number of forecasters, but GC should've pruned intermediary objects from step (1) and there should be room to do this in-memory.

Finally, evaluate the filter operations to see which can't be moved up sooner to make sure we're not processing a lot of data that is later pruned.

[tildechris]

Memory analysis

profvis doesn't give us great memory analysis, but there's two notable findings:

1. Not only is read_csv computationally expensive, but we're not reclaiming high % the memory allocated. This is another suggestion to break the script apart on a per-forecaster basis.
2. We do a ton of date comparisons. Not only does this take a lot of memory, but it takes a lot of time.

One recommendation from the second point is to review the datatypes used for date filtering to make sure we're not parsing strings and using actual date data types. If we are, the date arithmetic may be much quicker if we switch to milliseconds from epoc and use integer math.

[nmdefries]

This was the version of the code (or thereabouts) analyzed above.

[nmdefries]

We do a ton of date comparisons. Not only does this take a lot of memory, but it takes a lot of time.

One recommendation from the second point is to review the datatypes used for date filtering to make sure we're not parsing strings and using actual date data types. If we are, the date arithmetic may be much quicker if we switch to milliseconds from epoc and use integer math.

The slow date comparisons and frequent calls to is.Date are also evident in the current version of the pipeline code. The date columns (target_end_date and forecast_date) are Date types, so converting from character is not the issue.

The problem is loading lubridate. lubridate uses its own date comparison operators on Date types, in place of the base Date comparison operators. The lubridate Date methods are an order of magnitude (7x in benchmarking) slower apparently because of all the type-checking the method dispatcher does.

We use some lubridate functions in both the pipeline and in the dashboard, so this slowdown should be impacting both of them.

The pipeline is simple and it is easy to check ourselves that the date columns are indeed Date types so we don't get unexpected behavior. Then we can force the base operators to be used via

for (op in c("<", ">", "==", ">=", "<=", "!=")) {
  registerS3method(op, "Date", Ops.Date)
}

(Simply not loading lubridate may also work, but I'm having issues with that locally. It looks like using a lubridate function via :: also triggers the lubridate registerS3method call. The above is a stronger approach.)

Since the dashboard is much more complicated, it's not clear that we can force the use of base Date generics and get the same behavior. We could add in our own type-checking, but there are lots of opportunities for implicit type conversion, especially given the complex callback logic of the shiny app.

[nmdefries]

Speed comparison of lubridate and base date comparison functions.

aa <- as.Date("2022-01-01")
bb <- Sys.Date()

# Uses base comparison operator, `Ops.Date` `==`
for (op in c("<", ">", "==", ">=", "<=", "!=")) {
  registerS3method(op, "Date", Ops.Date)
}
bench::mark({
  aa == bb  
})
# # A tibble: 1 × 13
# expression        min   median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result    memory             time                gc                   
# <bch:expr>   <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>    <list>             <list>              <list>               
# { aa == bb }   2.33µs   4.27µs   173307.        0B     17.3  9999     1     57.7ms <lgl [1]> <Rprofmem [0 × 3]> <bench_tm [10,000]> <tibble [10,000 × 3]>


# Uses lubridate comparison operator, `comp_posix_date`
library(lubridate)
for (op in c("<", ">", "==", ">=", "<=", "!=")) {
  registerS3method(op, "Date", lubridate:::comp_posix_date)
}
bench::mark({
  aa == bb  
})
# # A tibble: 1 × 13
# expression        min   median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result    memory             time                gc                   
# <bch:expr>   <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>    <list>             <list>              <list>               
# { aa == bb }   18.2µs   20.3µs    43140.      328B     8.63  9998     2      232ms <lgl [1]> <Rprofmem [1 × 3]> <bench_tm [10,000]> <tibble [10,000 × 3]>

[nmdefries]

We could also store the date columns as character type. Date-style filtering works on characters, and it avoids repeatedly checking the column type and converting to character. Seems safer than using registerS3method, too.