The current README has a lot to be desired for new users. It is admittedly pretty overwhelming looking at the front page. There are few core issues to be addressed:

1. The "Simple Use Case" section should be an actual simple use case. While I think the information there is valuable and perhaps could be in a vignette, or perhaps a separate online web documentation, there is really no need to know about the full class information on our frontpage README.

• Move all discussion of classes to a suitable location
• Make data objects self-contained and load with package (if they are that critical)
• Switch to a sleep-wake binary vector as input, to avoid need for all the additional arguments in parse_sleeptimes
• Remove all discussion of object types, and have it divided into 2 clear steps: parse_sleeptimes, FIPS_simulate.

2. It is not clearly stated anywhere what the output columns from each model's FIPS_simulation is.

• We should have a simple table that says what the core time-varying predictions/parameter outputs are (i.e., c, s, etc...). This will be crucial for when we develop the formula interface for users to specify this themselves.

3. I think it's fine to briefly mention the two data types supported, but let's move all information about these somewhere specific where we can give the required room and formatting to get these all together.

• Have a seperate webpage/source of information for understanding data types.

4. This is a more tentative suggestion, but I'm thinking we don't really need the "What are BMMs" up front either. We could summarise this in a simple sentence, and again direct the user to our papers or an appropriate website page for more information.

I've been doing some work on providing a simple formula interface to FIPS https://github.com/humanfactors/FIPS/tree/formula-proposal-interface. The logic of providing this interface is twofold:

1. Users can more readily adjust what the predicted latent variable is from the outset of model simulation (e.g. provided below);
2. It is more transparent to the users how our latent pred_vars are calculated, and what variable they should index

The prototype I've hacked together is far from complete or perfect. Currently the general interface is something like the following:

TPM.simulation.results = FIPS_simulate(
  FIPS_df = simulated.dataframe, # The FIPS_df
  modeltype = "TPM",         # three process model
  pvec = TPM_make_pvec(),      # parameter vector
  formula = alertness_with_w ~ s + c + u + w
  )

Like any big change I do want to minimize issues with legacy code. I envision that this formula argument will be optional, with the previous defaults being placed in when the user supplies no argument (that is how it currently works). However, in writing the basic logic for this feature, I have come across a few issues:

• Currently, the FIPS_Simulation class is assigned after the main loop of each of the models

  FIPS/R/simulation_threeprocessmodel.R

  Line 300 in ab1da48

  dat <- FIPS_simulation(dat, modeltype = "TPM", pvec = pvec, pred_stat = "alertness", pred_cols = TPM_cols)

  The issue with this approach is that attributes need to be continuously added to the object within the FIPS_Simulate function. This creates a lot of noise, and you end up changing object attributes in helper functions down the line:

  FIPS/R/formula_interface_rlang.R

  Lines 28 to 32 in 6e2ca5b

  	add_formula_vector <- function(.FIPS_sim, pred_vector, pred_name) {
  	.FIPS_sim[,pred_name] = pred_vector
  	attr(.FIPS_sim, "pred_cols") = c(attr(.FIPS_sim, "pred_cols"), pred_name)
  	return(.FIPS_sim)
  	}

• Ideally, we are setting all attributes like pred_stat at the time we create the object.
• What I'm thinking is that the main loops for the model functions should effectively do nothing but be similar to a model.frame whereby it returns only the essential elements for the model.
• We then will need constructor functions for each model type (e.g., FIPS_simulate_TPM) and these are called in the FIPS_simulate function. These functions will take in the arguments like formula all the data etc., and then produce the required object. The added benefit is that we simplify the role of the main loops.
• This will have side-effects for the plotting functions which will need to be examined, and likely also for validation.
• The only issue I have is how we might add in help for the formula, given it's model depdendent.

In any case, this is my notes from late on a Friday, so when I get round to implementing this Monday I can remember what the hell I was thinking!!! @lukestrickland

Bug description and expected behaviour

Non-standard column names for parse_sleeptimes results in error.

• Reproduce
• Add test for this case

CMDR_FIPS_Times = FIPS::parse_sleeptimes(
  sleeptimes = CMDR_MINIMAL,
  series.start = LAUNCH_DATE,
  series.end = END_DATE,
  roundvalue = 5,
  sleep.start.col = "sleep_start", # Nonstandard
  sleep.end.col = "sleep_end", # Nonstandard
  sleep.id.col = "Rest.Period" # Nonstandard

Error in seq.POSIXt(simulationstart, firstsleep - lubridate::minutes(expand_by),  : 
  'to' must be a "POSIXt" object
In addition: Warning messages:
1: In min(rounded.sleeptimes[[sleep.start.col]]) :
  no non-missing arguments to min; returning Inf
2: In max(rounded.sleeptimes[[sleep.end.col]]) :
 
 Error in seq.POSIXt(simulationstart, firstsleep - lubridate::minutes(expand_by),  : 
  'to' must be a "POSIXt" object 
8.
stop("'to' must be a \"POSIXt\" object") 
7.
seq.POSIXt(simulationstart, firstsleep - lubridate::minutes(expand_by), 
    by = emins) 
6.
seq(simulationstart, firstsleep - lubridate::minutes(expand_by), 
    by = emins) 
5.
eval_tidy(xs[[j]], mask) 
4.
tibble_quos(xs[!is.null], .rows, .name_repair) 
3.
tibble::tibble(datetime = seq(simulationstart, firstsleep - lubridate::minutes(expand_by), 
    by = emins), sleep.id = NA, wake_status = T) at data_sleeptimesformat.R#134
2.
generate_presleep_times(series.start, minimum.sleepstart, roundvalue) at data_sleeptimesformat.R#96
1.
FIPS::parse_sleeptimes(sleeptimes = CMDR_MINIMAL, series.start = LAUNCH_DATE, 
    series.end = END_DATE, roundvalue = 5, sleep.start.col = "sleep_start", 
    sleep.end.col = "sleep_end", sleep.id.col = "Rest.Period") 

I wanted to use a small roundvalue to check an integral against a numerical approximation. I tried to set roundvalue to a decimal, e.g.,0.5 to get predictions every 30 seconds, but I encountered an error. I also get an error for decimal roundvalues bigger than 1 (e.g., roundvalue=2.5)

I think at least we should have an error message explaining that roundvalue must be a whole number.

library(FIPS)
library(tidyverse)
# Simulation start date time (i.e., when you want first predictions to begin)
simulation.start = lubridate::ymd_hms('2018-05-01 00:00:00', tz = "Australia/Perth")
# Simulation end date time (i.e., when you want predictions to end)
simulation.end = lubridate::ymd_hms('2018-05-09 21:00:00', tz = "Australia/Perth")
# Sleep times, 5 hours per night, starting 23:00
example.sleeptimes <- tibble::tibble(
  sleep.start = seq(
    from = lubridate::ymd_hms('2018-05-01 02:00:00', tz = "Australia/Perth"),
    to = lubridate::ymd_hms('2018-05-09 16:00:00', tz = "Australia/Perth"),
    by = '24 hours'),
  sleep.end = sleep.start + lubridate::dhours(3),
  sleep.id = rank(sleep.start))

simulated.dataframe = parse_sleeptimes(
  sleeptimes = example.sleeptimes,
  series.start = simulation.start,
  series.end = simulation.end,
  sleep.start.col = "sleep.start",
  sleep.end.col = "sleep.end",
  sleep.id.col = "sleep.id",
  roundvalue = 0.5)

System Information

sessionInfo()
R version 3.6.2 (2019-12-12)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18363)

Matrix products: default

locale:
[1] LC_COLLATE=English_Australia.1252  LC_CTYPE=English_Australia.1252   
[3] LC_MONETARY=English_Australia.1252 LC_NUMERIC=C                      
[5] LC_TIME=English_Australia.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
 [1] forcats_0.5.0   stringr_1.4.0   dplyr_1.0.2     purrr_0.3.4     readr_1.3.1    
 [6] tidyr_1.1.2     tibble_3.0.3    ggplot2_3.3.2   tidyverse_1.3.0 FIPS_0.1.1     

loaded via a namespace (and not attached):
 [1] tidyselect_1.1.0 haven_2.2.0      lattice_0.20-38  colorspace_1.4-1
 [5] vctrs_0.3.4      generics_0.0.2   utf8_1.1.4       rlang_0.4.7     
 [9] pillar_1.4.6     glue_1.4.2       withr_2.3.0      DBI_1.1.0       
[13] dbplyr_1.4.3     modelr_0.1.7     readxl_1.3.1     lifecycle_0.2.0 
[17] munsell_0.5.0    gtable_0.3.0     cellranger_1.1.0 rvest_0.3.5     
[21] labeling_0.3     fansi_0.4.1      broom_0.5.6      Rcpp_1.0.5      
[25] backports_1.1.10 scales_1.1.1     checkmate_2.0.0  jsonlite_1.6.1  
[29] farver_2.0.3     fs_1.4.1         hms_0.5.3        digest_0.6.25   
[33] stringi_1.4.6    grid_3.6.2       cli_2.0.2        tools_3.6.2     
[37] magrittr_1.5     crayon_1.3.4     pkgconfig_2.0.3  ellipsis_0.3.1  
[41] xml2_1.2.2       reprex_0.3.0     lubridate_1.7.9  assertthat_0.2.1
[45] httr_1.4.1       rstudioapi_0.11  R6_2.4.1         nlme_3.1-142    
[49] compiler_3.6.2