Regenerate draw ids for bind_draws(<draws_rvar>, along = "chain")

[mjskay]

Summary

This PR closes #300. The underlying issue in #300 is that bind_draws(<draws_rvars>, along = "chain") did not re-generate draw ids, so the resulting object from split_chain(<draws_rvars>) (which uses bind_draws() could have multiple draws with th same id.

Notably, this bug might have been caught more easily if there was a warning issued by order_draws(<rvar>) if it needs to merge chains. I added a simple warning using warn_merge_chains(), but with the default settings that warning is not issued anyway. So I'm not sure if we want some other long-term solution, maybe warning levels or something?

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[codecov-commenter]

Codecov Report

Merging #306 (e80ce44) into master (7868fcf) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head e80ce44 differs from pull request most recent head e551e5d. Consider uploading reports for the commit e551e5d to get more accurate results

@@           Coverage Diff           @@
##           master     #306   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files          47       47           
  Lines        3683     3686    +3     
=======================================
+ Hits         3521     3524    +3     
  Misses        162      162           

Files	Coverage Δ
R/bind_draws.R	97.00% <100.00%> (+0.03%)	:arrow_up:
R/order_draws.R	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions[bot]]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if e80ce44355d44e7b7244ef15247a86bdd3d246f7 is merged into master:

•   :ballot_box_with_check:as_draws_array: 181ms -> 177ms [-4.95%, +0.94%]
•   :ballot_box_with_check:as_draws_df: 61.3ms -> 62.7ms [-4.47%, +9.26%]
•   :ballot_box_with_check:as_draws_list: 321ms -> 324ms [-0.56%, +2.64%]
•   :ballot_box_with_check:as_draws_matrix: 58.1ms -> 56.9ms [-6.28%, +2.25%]
•   :ballot_box_with_check:as_draws_rvars: 291ms -> 296ms [-0.52%, +3.94%]
•   :ballot_box_with_check:summarise_draws_100_variables: 1.28s -> 1.26s [-3.88%, +0.67%]
•   :ballot_box_with_check:summarise_draws_10_variables: 139ms -> 139ms [-2.53%, +3.68%] Further explanation regarding interpretation and methodology can be found in the documentation.

[paul-buerkner]

Thank you very much! A long term solution of warning levels sounds sensible. For this, I guess, we have to gather all potential warnings and decide which to issue by default and which to issue at different levels of complaininess. Anyway, for now, I will merge this PR, run all posterior and then release a new version to CRAN today.