Return time in read_cmdstan_csv for MCMC#414
Conversation
Codecov Report
@@ Coverage Diff @@
## master #414 +/- ##
==========================================
+ Coverage 88.19% 88.29% +0.10%
==========================================
Files 12 12
Lines 2769 2793 +24
==========================================
+ Hits 2442 2466 +24
Misses 327 327
Continue to review full report at Codecov.
|
jgabry
left a comment
jgabry
left a comment
There was a problem hiding this comment.
I made a few small changes and added review comments explaining them. If you like them then we can go ahead and merge this. If not then we can revert my changes.
| #' * `total`: (scalar) The total run time. For MCMC this may be different than | ||
| #' the sum of the chain run times if parallelization was used. |
There was a problem hiding this comment.
I added this clarification in the fit$time() doc
| } | ||
| list( | ||
| metadata = metadata, | ||
| time = list(total = NA_integer_, chains = time), |
There was a problem hiding this comment.
this keeps the structure consistent with the output of fit$time() but total is NA because we can't infer it from the CSV files
| #' * `time`: Run time information for the individual chains. The returned object | ||
| #' is the same as for the [$time()][fit-method-time] method except the total run | ||
| #' time can't be inferred from the CSV files (the chains may have been run in | ||
| #' parallel) and is therefore `NA`. |
There was a problem hiding this comment.
Added some doc for the time component of the returned list
|
Thanks. I agree with all changes. |
3 participants
Summary
read_cmdstan_csv now returns time if MCMC CSVs are used.
Copyright and Licensing
Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Rok Češnovar
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses: