Skip to content

Commit

Permalink
skip data validation to speed up generation
Browse files Browse the repository at this point in the history
  • Loading branch information
@lunafazio
lunafazio committed Jul 4, 2023
1 parent 373a4c2 commit 4e29cd5
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion R/generator-brms.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ brms_full_ppred <- function(fit, newdata = NULL, draws = NULL) {
pp_data[[i]][, vars] <- array(
brms::posterior_predict(
fit, newdata = pp_data[[i]],
resp = vars, draw_ids = i),
resp = vars, draw_ids = i,
skip_validate = TRUE),
dim = c(1, n, length(vars)))[1,,]
}
}
Expand Down

3 comments on commit 4e29cd5

@lunafazio
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only situation where I anticipate this might be risky is when factor variables are involved as levels should not be dropped/reordered. Should be easy to test by simulating data from something with an intermediate factor variable with some low probability outcomes to ensure they are all not present in every generation.

E.g. (factor_outcome ~ x) + (y ~ factor_outcome)

@martinmodrak
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think this is safe - brms should always keep the original data/fit structure as a reference and not reorder anything in posterior predict.

@lunafazio
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what I thought! However, after doing some quick tests, it seems that factor responses are returned as a numeric vector + levels attribute but no factor class, so brms won't accept its own output in the brms_full_ppred loop. And if two or more responses are requested, it even drops the levels. Of course this is something to be fixed from the brms side, but just heads up in case some error pops up around this. As a workaround, I found it is possible to use a factor where the level names are exactly the numbers used to encode values.

Luckily since this is only a problem when generating data with something depending on a factor, the introduction of brms_full_ppred to SBC should not break anything that already worked previously.

Please sign in to comment.