Conversation
Replaced inefficient loop-based dataframe construction and `bind_rows` in `marg_pabs` with vectorized subsetting. This significantly improves performance and removes the dependency on `dplyr` for this function. The logic for `est_samples` (posterior draws) remains consistent with the original implementation: `est_samples` is flattened column-wise (draws for observation 1, then observation 2, etc.), which corresponds to the vectorized expansion of `newdata` (row 1 repeated n_sim times, then row 2 repeated n_sim times). This change improves performance by avoiding repeated memory allocations and dataframe binding operations. It also makes the function more robust by relying on base R instead of the suggested `dplyr` package. Co-authored-by: lcgodoy <24809942+lcgodoy@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
Optimized
marg_pabsinR/utils.Rby replacing the loop and list growth with vectorized operations.Why: The original implementation used a loop to create a list of dataframes (one for each row of
newdata, replicatedn_simtimes) and then bound them usingdplyr::bind_rows. This is inefficient for large datasets or simulations.What:
newdata[rep(seq_len(n_obs), each = n_sim), , drop = FALSE].dplyr::mutatewith base R assignment:output$prob_abs <- ....dplyr(which is inSuggests).data.frameinstead of atibbleif the input was adata.frame(previouslybind_rowsforced atibble).Verification:
marg_pabsand does not affect other functions, althoughmarg_recandmarg_survshare similar patterns that could be optimized in future work.PR created automatically by Jules for task 17284846057200474667 started by @lcgodoy