summary: rework "incomplete run" check to minimize false positives

[kyleam]

This series fixes #293 and hopefully prevents us from running into more cases like #288 and #293. The first two commits tighten up the tests and parsing for the RunDetails.RunEnd field (which is where the "Stop Time:" value is recorded). The third commit is the main change, and the last commit adds a regression test for the case in #293.

Thanks to @seth127 for the suggestion to use "Stop Time:" in the lst file as a signal that the run is complete.

cc: @callistosp @kylebaron

[kyleam]

bbr's test suite passes with a bbi built from this PR.

ip-10-128-19-181:bbr$ git rev-parse HEAD
9b77195df632ad58e6ae2761dfdbc3f5c40a7efb
ip-10-128-19-181:bbr$ which bbi
/data/home/kylem/go/bin/bbi
ip-10-128-19-181:bbr$ export BBI_EXE_PATH=$(which bbi)
ip-10-128-19-181:bbr$ export BBR_DEV_NO_MIN_VERSION=1

check output

``` ══ Documenting ═════════════════════════════════════════════════════════════════ ℹ Installed roxygen2 version (7.2.1) doesn't match required (7.2.2) ✖ `check()` will not re-document this package ══ Building ════════════════════════════════════════════════════════════════════ Setting env vars: • CFLAGS : -Wall -pedantic • CXXFLAGS : -Wall -pedantic • CXX11FLAGS: -Wall -pedantic • CXX14FLAGS: -Wall -pedantic • CXX17FLAGS: -Wall -pedantic • CXX20FLAGS: -Wall -pedantic * checking for file ‘/data/home/kylem/src/github/metrumresearchgroup/bbr/DESCRIPTION’ ... OK * preparing ‘bbr’: * checking DESCRIPTION meta-information ... OK * installing the package to build vignettes * creating vignettes ... OK * checking for LF line-endings in source and make files and shell scripts * checking for empty or unneeded directories Omitted ‘LazyData’ from DESCRIPTION * building ‘bbr_1.4.0.8000.tar.gz’ ══ Checking ════════════════════════════════════════════════════════════════════ Setting env vars: • _R_CHECK_CRAN_INCOMING_USE_ASPELL_ : TRUE • _R_CHECK_CRAN_INCOMING_REMOTE_ : FALSE • _R_CHECK_CRAN_INCOMING_ : FALSE • _R_CHECK_FORCE_SUGGESTS_ : FALSE • _R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_: FALSE • NOT_CRAN : true ── R CMD check ───────────────────────────────────────────────────────────────── * using log directory ‘/tmp/RtmphplSQj/file7b3f2fd7c21a/bbr.Rcheck’ * using R version 4.1.3 (2022-03-10) * using platform: x86_64-pc-linux-gnu (64-bit) * using session charset: UTF-8 * using options ‘--no-manual --as-cran’ * checking for file ‘bbr/DESCRIPTION’ ... OK * checking extension type ... Package * this is package ‘bbr’ version ‘1.4.0.8000’ * package encoding: UTF-8 * checking package namespace information ... OK * checking package dependencies ... NOTE Imports includes 21 non-default packages. Importing from so many packages makes the package vulnerable to any of them becoming unavailable. Move as many as possible to Suggests and use conditionally. * checking if this is a source package ... OK * checking if there is a namespace ... OK * checking for executable files ... WARNING Found the following executable file: bin/bbi Source packages should not contain undeclared executable files. See section ‘Package structure’ in the ‘Writing R Extensions’ manual. * checking for hidden files and directories ... OK * checking for portable file names ... OK * checking for sufficient/correct file permissions ... OK * checking whether package ‘bbr’ can be installed ... OK * checking installed package size ... OK * checking package directory ... OK * checking for future file timestamps ... OK * checking ‘build’ directory ... OK * checking DESCRIPTION meta-information ... OK * checking top-level files ... NOTE Non-standard files/directories found at top level: ‘bin’ ‘check-output’ ‘scratch.R’ ‘test-output’ * checking for left-over files ... OK * checking index information ... OK * checking package subdirectories ... OK * checking R files for non-ASCII characters ... OK * checking R files for syntax errors ... OK * checking whether the package can be loaded ... OK * checking whether the package can be loaded with stated dependencies ... OK * checking whether the package can be unloaded cleanly ... OK * checking whether the namespace can be loaded with stated dependencies ... OK * checking whether the namespace can be unloaded cleanly ... OK * checking loading without being on the library search path ... OK * checking dependencies in R code ... OK * checking S3 generic/method consistency ... OK * checking replacement functions ... OK * checking foreign function calls ... OK * checking R code for possible problems ... OK * checking Rd files ... OK * checking Rd metadata ... OK * checking Rd line widths ... OK * checking Rd cross-references ... OK * checking for missing documentation entries ... OK * checking for code/documentation mismatches ... OK * checking Rd \usage sections ... OK * checking Rd contents ... OK * checking for unstated dependencies in examples ... OK * checking installed files from ‘inst/doc’ ... OK * checking files in ‘vignettes’ ... OK * checking examples ... OK * checking for unstated dependencies in ‘tests’ ... OK * checking tests ... Running ‘testthat.R’ [198s/98s] OK * checking for unstated dependencies in vignettes ... OK * checking package vignettes in ‘inst/doc’ ... OK * checking re-building of vignette outputs ... OK * checking for non-standard things in the check directory ... OK * checking for detritus in the temp directory ... NOTE Found the following files/directories: ‘cc4BPpl6.s’ ‘ccjtS0k6.s’ ‘cct5kVl6.s’ ‘ccT7ilj6.s’ ‘ccU8cyk6.s’ ‘ccWD6tC2.s’ * DONE Status: 1 WARNING, 3 NOTEs See ‘/tmp/RtmphplSQj/file7b3f2fd7c21a/bbr.Rcheck/00check.log’ for details. ── R CMD check results ───────────────────────────────────── bbr 1.4.0.8000 ──── Duration: 2m 37.9s ❯ checking for executable files ... WARNING Found the following executable file: bin/bbi Source packages should not contain undeclared executable files. See section ‘Package structure’ in the ‘Writing R Extensions’ manual. ❯ checking package dependencies ... NOTE Imports includes 21 non-default packages. Importing from so many packages makes the package vulnerable to any of them becoming unavailable. Move as many as possible to Suggests and use conditionally. ❯ checking top-level files ... NOTE Non-standard files/directories found at top level: ‘bin’ ‘check-output’ ‘scratch.R’ ‘test-output’ ❯ checking for detritus in the temp directory ... NOTE Found the following files/directories: ‘cc4BPpl6.s’ ‘ccjtS0k6.s’ ‘cct5kVl6.s’ ‘ccT7ilj6.s’ ‘ccU8cyk6.s’ ‘ccWD6tC2.s’ 0 errors ✔ | 1 warning ✖ | 3 notes ✖ Error: R CMD check found WARNINGs Execution halted ```

[callistosp]

Thanks for verifying @seth127. Yes, this would be the expected behavior. If you run additional estimations within the same run, typically this is done to improve the outcome on subsequent runs, or to estimate the objective function value. You still want to be able to look at the results of the lead-in estimation methods to make sure something didn't go wrong, but the relevant results will always be the outputs of the final estimation method. Plus, it is my understanding that any heuristics from any of the estimation methods in the run will show up in the model_summary output, so this is mainly affecting the parameter estimates that are being parsed by bbr

[callistosp]

if @kylebaron or @timwaterhouse can think of an edge case where the above statements are not true I would love to hear it!

[kyleam]

@callistosp

Plus, it is my understanding that any heuristics from any of the estimation methods in the run will show up in the model_summary output [...]

Yes, I think that's right. As far as I can tell, all of bbi's heuristics output comes from looking at the lst file.

[kyleam]

Merging, as the behavior being discussed wasn't introduced by this PR (just highlighted by the new tests), but please don't let that discourage any further discussion (and possibly creation of a dedicated issue, if it turns out there is something we want to change here).