diff --git a/.jules/bolt.md b/.jules/bolt.md index f658475..470990a 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -15,3 +15,10 @@ ## 2024-05-15 - [R Performance: ifelse Overhead] **Learning:** In R, ifelse evaluates both true and false branches entirely before subsetting, which is very inefficient for vector operations. **Action:** Optimize this by preallocating with res <- Y * 0 to preserve attributes and using vectorized subsetting like if any cond res subset <- ... + +## 2024-05-14 - Patch formatting and `ifelse` subsetting safety +**Learning:** When using `replace_with_git_merge_diff` to add lines with comments, the diff block must strictly mirror the original context lines. Simply injecting a line like `## Bolt: replaced apply...` into what otherwise looks like unmodified `SEARCH` block context will cause patch application to fail, because standard `patch` tools expect precise context matching. Additionally, when optimizing `ifelse(cond, true_val, false_val)` in R, using `res[which(cond)] <- ...` is not only slightly faster than `res[cond & !is.na(cond)] <- ...` but also inherently safer, as it idiomatically omits `NA` indices without raising 'NAs are not allowed' errors. +**Action:** Always ensure that any new comments or modifications intended for the file are placed correctly within the `REPLACE` block and not hallucinated as unmodified context in the `SEARCH` block. When replacing `ifelse` with subsetting where the condition might contain `NA`, prefer `which(cond)` to safely isolate the indices. +## 2024-05-14 - ifelse() attribute drops and test contracts +**Learning:** While `res <- y/n; res[which(n == 0)] <- 0` is faster than `ifelse(n == 0, 0, y/n)`, `ifelse` intrinsically drops array dimensions/names if the test condition does not have them, and returns a scalar if the condition is a scalar. In `nonnest2`, the log-likelihood weight calculation relies heavily on this attribute stripping to match expected shapes. When vectorized replacement was used, `wt_res` retained names when it shouldn't have, breaking exact tests (`test_llcont_binomial_weight_contract.R`). +**Action:** When replacing `ifelse()` with vectorized subsetting in R packages with stringent unit tests, carefully inspect whether downstream code relies on the specific attribute-stripping behavior of `ifelse`. If the performance gain is negligible (microseconds on vectors of length 1 or 2) compared to loop vectorization (like `apply` to `rowSums`), leave `ifelse` intact to preserve strict test contracts. diff --git a/R/llcont.R b/R/llcont.R index d8e496a..f462ee6 100644 --- a/R/llcont.R +++ b/R/llcont.R @@ -53,12 +53,12 @@ llcont.glm <- function(x, ...){ if(is.matrix(y)) { ## Bolt: replaced apply(..., 1, sum) with optimized rowSums() for performance n <- rowSums(y) - y <- ifelse(n == 0, 0, y[, 1]/n) + y <- ifelse(n == 0, 0, y[, 1]/n) } else { n <- rep.int(1, length(y)) } m <- if (any(n > 1)) n else wt - wt <- ifelse(m > 0, (wt/m), 0) + wt <- ifelse(m > 0, (wt/m), 0) dbinom(round(m * y), round(m), mpreds, log = TRUE) * wt }, quasibinomial = { @@ -407,7 +407,8 @@ llcont.lavaan <- function(x, ...){ if(tolower(lavInspect(x, "options")$missing) == "ml.x") stop("cannot handle lavaan models with missing='ml.x'. consider using missing='ml'.", call. = FALSE) mispatts <- lavInspect(x, "patterns") if(any(class(mispatts) == "list")){ - npatts <- max(sapply(mispatts, nrow)) + ## Bolt: replaced sapply with vapply for performance and safety + npatts <- max(vapply(mispatts, nrow, FUN.VALUE = 1L)) } else { npatts <- nrow(mispatts) }