search

stan-dev / posterior

The posterior R package
https://mc-stan.org/posterior/
Other
167 stars 24 forks source link

Converting from older formats #62

Closed mtwest2718 closed 4 years ago

mtwest2718 commented 4 years ago

Resurrecting the question addressed in #16 as I am new and think its more feasible if we write only one way transforms (to Posterior). Investigating the coda archive and changelog, very little of the code has changed in the past five years. Also the functions that handle mcmc objects as well as BUGS & JAGS files do not require external libraries.

I would argue that the value of an expanded user base for this library and its downstream tools outweighs the code maintenance cost.

paul-buerkner commented 4 years ago

I didn't add a comment on why I closed #16 but if I remember it correctly, it was because working with them would have required importing coda which we want to avoid. If we can find a way to support them in some way without importing coda, I agree we should consider it.

mtwest2718 commented 4 years ago

Summary of afternoon conversation with @paul-buerkner

jgabry commented 4 years ago

Sounds good, thanks @mtwest2718!

mtwest2718 commented 4 years ago

Playing around with the output this rJAGS example, the conversion from mcmc object to any of the draws formats works as is without any changes. This may be due to small blocks found in each of the _asdraws* R files like the below (_as_drawslist.R lines 64-68).

#' @rdname draws_list
#' @export
as_draws_list.mcmc <- function(x, ...) {
  as_draws_list(as_draws_matrix(x), ...)
}

The mcmc.list method is also in the code base

#' @rdname draws_list
#' @export
as_draws_list.mcmc.list <- function(x, ...) {
  as_draws_list(as_draws_array(x), ...)
}

and looks to be working alright. Running git log as_draws_df.R, the only possible pointer is commit 52c06dcac0759c2d12193fa708676f2806193027 regarding issue #16 by @paul-buerkner from last November. While the issue was closed seemingly in the negative, the draft code was never removed from the repo. I would like to do some more testing just to make sure results structurally match what they should look like if we were transforming rstanarm samples.

There may be a development process issue here if there is functionality lurking in the codebase that our resolutions suggest shouldn't exist, but I'd like to take that to Slack or email.

paul-buerkner commented 4 years ago

Nice! I totally forgot about that but apparently we did resolve the issue by implementing these functions so that's great!

WestByNoreaster notifications@github.com schrieb am Mo., 2. März 2020, 13:55:

Playing around with the output this rJAGS example, the conversion from mcmc object to any of the draws formats works as is without any changes. This may be due to small blocks found in each of the *as_draws* R files like the below (as_draws_list.R* lines 64-68).

' @rdname draws_list

' @export

as_draws_list.mcmc <- function(x, ...) { as_draws_list(as_draws_matrix(x), ...) }

The mcmc.list method is also in the code base

' @rdname draws_list

' @export

as_draws_list.mcmc.list <- function(x, ...) { as_draws_list(as_draws_array(x), ...) }

and looks to be working alright. Running git log as_draws_df.R, the only possible pointer is commit 52c06dc https://github.com/jgabry/posterior/commit/52c06dcac0759c2d12193fa708676f2806193027 regarding issue #16 https://github.com/jgabry/posterior/issues/16 by @paul-buerkner https://github.com/paul-buerkner from last November. While the issue was closed seemingly in the negative, the draft code was never removed from the repo. I would like to do some more testing just to make sure results structurally match what they should look like if we were transforming rstanarm samples.

There may be a development process issue here if there is functionality lurking in the codebase that our resolutions suggest shouldn't exist, but I'd like to take that to Slack or email.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgabry/posterior/issues/62?email_source=notifications&email_token=ADCW2AAIX446ZSQ5NUALCKDRFOUCTA5CNFSM4K3GTYY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENPGAXQ#issuecomment-593387614, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCW2ACDYIJ2C4RO4EEAKTTRFOUCTANCNFSM4K3GTYYQ .

paul-buerkner commented 4 years ago

It as I see the code I now remember that it was likely me implementing these functions so at least we know who to blame :-D

Paul Buerkner paul.buerkner@gmail.com schrieb am Mo., 2. März 2020, 15:57:

Nice! I totally forgot about that but apparently we did resolve the issue by implementing these functions so that's great!

WestByNoreaster notifications@github.com schrieb am Mo., 2. März 2020, 13:55:

Playing around with the output this rJAGS example, the conversion from mcmc object to any of the draws formats works as is without any changes. This may be due to small blocks found in each of the *as_draws* R files like the below (as_draws_list.R* lines 64-68).

' @rdname draws_list

' @export

as_draws_list.mcmc <- function(x, ...) { as_draws_list(as_draws_matrix(x), ...) }

The mcmc.list method is also in the code base

' @rdname draws_list

' @export

as_draws_list.mcmc.list <- function(x, ...) { as_draws_list(as_draws_array(x), ...) }

and looks to be working alright. Running git log as_draws_df.R, the only possible pointer is commit 52c06dc https://github.com/jgabry/posterior/commit/52c06dcac0759c2d12193fa708676f2806193027 regarding issue #16 https://github.com/jgabry/posterior/issues/16 by @paul-buerkner https://github.com/paul-buerkner from last November. While the issue was closed seemingly in the negative, the draft code was never removed from the repo. I would like to do some more testing just to make sure results structurally match what they should look like if we were transforming rstanarm samples.

There may be a development process issue here if there is functionality lurking in the codebase that our resolutions suggest shouldn't exist, but I'd like to take that to Slack or email.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgabry/posterior/issues/62?email_source=notifications&email_token=ADCW2AAIX446ZSQ5NUALCKDRFOUCTA5CNFSM4K3GTYY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENPGAXQ#issuecomment-593387614, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCW2ACDYIJ2C4RO4EEAKTTRFOUCTANCNFSM4K3GTYYQ .

mtwest2718 commented 4 years ago

git blame paul

Before I close this out, I want to test a few more packages that produce output using coda, just to make sure things work as desired/expected. Is there any place to put the testing script?

paul-buerkner commented 4 years ago

You could put it in a folder under tests/ and make sure to add it to .Rbuildignore (I forgot the exact file name right now) so that it is not included in the built R package.

WestByNoreaster notifications@github.com schrieb am Di., 3. März 2020, 15:23:

git blame paul

Before I close this out, I want to test a few more packages that produce output using coda, just to make sure things work as desired/expected. Is there any place to put the testing script?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgabry/posterior/issues/62?email_source=notifications&email_token=ADCW2ACBYV5UB6YFFR5RU5DRFUHGLA5CNFSM4K3GTYY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENTVXOI#issuecomment-593976249, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCW2AB6AMQ5MT3DSGMXY6LRFUHGLANCNFSM4K3GTYYQ .

paul-buerkner commented 4 years ago

Is there anything that needs to be done before we can close this issue?

mtwest2718 commented 4 years ago

No we are good.