Refactoring Suggestion: Unify Progress Tracking with progressr Package

[mronkko]

This is written by Claude Code:

Refactoring Suggestion: Unify Progress Tracking with progressr Package

Summary

The current implementation uses pbapply for serial and parallel execution paths, while the future execution path uses progressr. I suggest refactoring to use progressr consistently across all execution paths for better maintainability and consistency.

Current State

The codebase currently has three different progress tracking approaches:

1. Serial path (R/analysis.R:38-73): Uses pbapply::pblapply()
2. Parallel cluster path (R/analysis.R:84-97): Uses pbapply::pblapply(cl=cl)
3. Future path (R/analysis.R:14-35): Uses progressr::progressor() ✓

Rationale for Unification

Benefits of using progressr everywhere:

1. Consistency: Single progress tracking mechanism across all execution modes
2. Already imported: progressr is already a dependency (DESCRIPTION:35)
3. Proven to work: Future path demonstrates it works well in practice
4. Better flexibility: Users can customize progress handlers via progressr::handlers()
5. File-friendly by design: progressr handles non-interactive output naturally
6. Reduced dependencies: Could potentially remove pbapply dependency entirely
7. Maintainability: One codebase to maintain instead of two different systems

Proposed Changes

Replace pbapply::pblapply() calls with progressr-based implementation similar to the future path:

Serial path (currently lines 38-73):

# Instead of:
results <- if(progress){
    try(pbapply::pblapply(1L:replications, used_mainsim, ...))
} else {
    try(lapply(1L:replications, used_mainsim, ...))
}

# Use:
iters <- 1L:replications
if(progress) {
    p <- progressr::progressor(along = iters)
} else {
    p <- NULL
}
results <- try(lapply_with_progress(iters, used_mainsim, p = p, ...))

# Where lapply_with_progress is:
lapply_with_progress <- function(X, FUN, p = NULL, ...) {
    results <- vector("list", length(X))
    for(i in seq_along(X)) {
        results[[i]] <- FUN(X[[i]], ...)
        if(!is.null(p)) p()
    }
    results
}

Parallel path: Similar adaptation using parallel::parLapply() with progressor updates on the main process.

Backward Compatibility

• No user-facing API changes required
• progress parameter continues to work as before
• Users who already customize progressr::handlers() see no change
• Interactive users still get progress bars (via progressr's default handlers)

Migration Path

1. Phase 1: Add progressr implementation alongside existing pbapply code
2. Phase 2: Test thoroughly across all execution modes
3. Phase 3: Deprecate pbapply usage (keep as fallback)
4. Phase 4: Remove pbapply dependency in future major version

Alternative: Keep Current Hybrid Approach

If maintaining pbapply is preferred:

Pros:

• No refactoring needed
• pbapply is mature and stable
• Current fix (recent commit) already solves the non-interactive issue

Cons:

• Two different progress systems to maintain
• Inconsistent user experience between execution modes
• Future path already proves progressr is sufficient

Implementation Effort

• Low-Medium: Core logic already exists in future path
• Most work is adapting serial/parallel paths to use similar pattern
• Extensive testing needed across execution modes

Note: This is a long-term architectural suggestion and not urgent. The recent fix already addresses the immediate need for SLURM cluster progress tracking.

[philchalmers]

I'm fine with the current hybrid approach. Switching to only one system is not really on my radar as all implementations come with their own limitations and front-end user demands, which in the initial exposure to the package should be kept minimum. While I agree there is redundancy in the internal code-base simply for the purpose of parallel distributions, I don't see this as a major issue. Recommend closing.