poissonconsulting / nlist

An R package to create and check numeric list objects
https://poissonconsulting.github.io/nlist/
Other
6 stars 1 forks source link

Speed up `as_nlists.mcmc()` as real bottle neck #31

Closed joethorley closed 1 year ago

joethorley commented 3 years ago

Consider following code

nlist::as_nlists(nlist::as_mcmc(nlist::as_mcmc_list(mcmcr::mcmcr_example)))
joethorley commented 1 year ago

This is fast

as_nlists(as.mcmcr(as.mcmc(collapse_chains(analysis$mcmcr))))
joethorley commented 1 year ago

This is not

as_nlists(as.mcmc(collapse_chains(analysis$mcmcr)))
krlmlr commented 1 year ago

The first variant splits the data by row and then creates an nlist object for each row. This is slow because the same expensive verification of the columns (=parameters) has to be made for each row.

We can surely require the mcmcr package to be installed for this to work. Either implement in mcmcr and remove the implementation here, or add mcmcr to "Suggests" and forward to mcmcr.

library(nlist)

mcmc_list <- as_mcmc_list(mcmcr::mcmcr_example)
mcmc <- as_mcmc(mcmc_list)

bench::mark(
  as_nlists(mcmc),
  as_nlists(mcmcr::as.mcmcr(mcmc))
)
#> Warning: Some expressions had a GC in every iteration; so filtering is
#> disabled.
#> # A tibble: 2 × 6
#>   expression                            min  median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>                       <bch:tm> <bch:t>     <dbl> <bch:byt>    <dbl>
#> 1 as_nlists(mcmc)                        8s      8s     0.125    10.6MB     32.6
#> 2 as_nlists(mcmcr::as.mcmcr(mcmc))   36.2ms  38.1ms    25.9     526.8KB     46.2

Created on 2023-05-19 with reprex v2.0.2

joethorley commented 1 year ago

nlist is a dependency of mcmcr. Is it OK to also have mcmcr in the Suggests?

krlmlr commented 1 year ago

That works, pillar suggests and also is a strong dependency of tibble.

Will as_nlists() ever be called for such objects when mcmcr is not loaded? If not, we wouldn't need to introduce the loop, and could move the implementation (and tests) to the mcmcr package.

joethorley commented 1 year ago

I'm not sure. I transferred a bunch of these functions from mcmcr to nlist two minor versions ago.

# mcmcr 0.4.0

- Added `as_mcmc_list.mcmr()`.
- Moved the following to `nlist` 
  - `as_nlist.mcmc()` and `as_nlist.mcmc.list()`
  - `as_nlists.mcmc()`
  - `as.term.mcmc()` and `as.term.mcmc.list()`
  - `bind_iterations.mcmc()` and `bind_iterations.mcmc.list()`
  - `collapse_chains.default()` and `collapse_chains.mcmc.list()`

I think I like the loop idea or a third idea is to code up a faster version in nlist.