From 801f4085c6fefab931d190d373660e777e60254e Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Tue, 9 Jun 2026 22:04:10 -0400 Subject: [PATCH 1/7] Add ignore argument to FORLOOP --- R/FORLOOP.R | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/R/FORLOOP.R b/R/FORLOOP.R index 0f3f728..383b372 100644 --- a/R/FORLOOP.R +++ b/R/FORLOOP.R @@ -11,6 +11,7 @@ #' #' @param code The right-hand side of a parameter declaration #' @param modelInfo Used internally by nimbleMacros; a list of model information such as constants and dimensions +#' @param ignore Index ranges to ignore when creating for loops; must be provided as a list of quoted code #' @param .env Used internally by nimbleMacros; the environment where the model was created #' #' @return NIMBLE code for a for loop or series of nested for loops. @@ -27,7 +28,11 @@ NULL #' @export FORLOOP <- nimble::buildMacro( -function(code, modelInfo, .env){ +function(code, modelInfo, ignore = NULL, .env){ + # Super clunky way of extracting the ignore argument + # seems to be necessary to do it this way given buildMacro options + ignore <- eval(code[[length(code)]]$ignore) + code[[length(code)]]$ignore <- NULL # Remove FORLOOP from the code line code <- removeMacroCall(code) LHS <- getLHS(code) @@ -49,7 +54,7 @@ function(code, modelInfo, .env){ idx_letters <- lapply(1:length(idx_sub), function(i) modelInfo$indexCreator()) idx_letters <- lapply(idx_letters, as.name) # Replace the ranges with the new corresponding letters - code <- replaceDeclarationIndexRanges(code, idx_letters) + code <- replaceDeclarationIndexRanges(code, idx_letters, ignore) # Then insert the ranges into the for loop structure idx_sub <- replaceRanges(idx_sub, idx_letters) # So e.g. @@ -168,13 +173,16 @@ removeIndexAdjustments <- function(idx){ # Check if index range on both sides of declaration are the same # Specifically, all indices on RHS have to be present in LHS -hasMatchingIndexRanges <- function (LHS, RHS){ +# ignore must be a list of quoted code e.g. list(quote(1:n)) +hasMatchingIndexRanges <- function (LHS, RHS, ignore = NULL){ idx_LHS <- extractIndices(LHS) idx_LHS <- idx_LHS[isIndexRange(idx_LHS)] idx_LHS <- lapply(idx_LHS, removeIndexAdjustments) idx_RHS <- extractAllIndices(RHS) idx_RHS <- idx_RHS[isIndexRange(idx_RHS)] idx_RHS <- lapply(idx_RHS, removeIndexAdjustments) + idx_RHS <- idx_RHS[!idx_RHS %in% ignore] + if(length(idx_RHS) == 0) return(TRUE) all(idx_RHS %in% idx_LHS) } @@ -223,11 +231,13 @@ recursiveReplaceIndex <- function(code, old_idx, new_idx){ } # Replace all indices on both sides LHS/RHS -replaceDeclarationIndexRanges <- function(code, new_idx_list){ +# ignore, if provided, must be a list of quoted code +# e.g. list(quote(1:n), quote(1:3)) +replaceDeclarationIndexRanges <- function(code, new_idx_list, ignore = NULL){ LHS <- getLHS(code) RHS <- getRHS(code) op <- code[[1]] - if(!hasMatchingIndexRanges(LHS, RHS)) stop("Index ranges must match") + if(!hasMatchingIndexRanges(LHS, RHS, ignore)) stop("Index ranges must match") idx <- extractAllIndices(LHS) idx <- idx[isIndexRange(idx)] if(length(idx) == 0) return(code) From a4fee793af59dc6fc11dd80ffe369fcb0faad4ec Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Tue, 9 Jun 2026 22:06:49 -0400 Subject: [PATCH 2/7] Make sure simplifyIndices ignores any index ranges --- R/utilities.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/utilities.R b/R/utilities.R index 3135484..d27bdbd 100644 --- a/R/utilities.R +++ b/R/utilities.R @@ -195,6 +195,8 @@ simplifyIndices <- function(code, new_indices){ if(is.name(x) | is.symbol(x)) return(x) if(x[[1]] == "for"){ unique_idx <- unique(extractAllIndices(x)) + # Ignore any remaining index *ranges* + unique_idx <- unique_idx[sapply(unique_idx, is.name)] # Ignore blank indices as in x[i,] unique_idx <- unique_idx[sapply(unique_idx, function(x) x!="")] if(length(unique_idx) > length(new_indices)){ From de0594071da836955fe998a834c693142bc58046 Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Wed, 10 Jun 2026 09:16:19 -0400 Subject: [PATCH 3/7] Handle ignore on LHS and add better error handling --- R/FORLOOP.R | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/R/FORLOOP.R b/R/FORLOOP.R index 383b372..3a8f372 100644 --- a/R/FORLOOP.R +++ b/R/FORLOOP.R @@ -28,11 +28,21 @@ NULL #' @export FORLOOP <- nimble::buildMacro( -function(code, modelInfo, ignore = NULL, .env){ +function(code, modelInfo, ignore = list(), .env){ # Super clunky way of extracting the ignore argument # seems to be necessary to do it this way given buildMacro options ignore <- eval(code[[length(code)]]$ignore) + # necessary for when ignore is not explicitly included in arguments + if(is.null(ignore)) ignore <- list() + # get rid of any ignore stuff in code code[[length(code)]]$ignore <- NULL + # Check ignore structure + if(!is.list(ignore)) stop("ignore argument must be a list", call.=FALSE) + if(length(ignore) > 0){ + is_call <- sapply(ignore, is.call) + if(!all(is_call)) stop("Some elements of ignore are not quoted code/calls", call.=FALSE) + } + # Remove FORLOOP from the code line code <- removeMacroCall(code) LHS <- getLHS(code) @@ -50,6 +60,8 @@ function(code, modelInfo, ignore = NULL, .env){ # Subset only indexes with ranges idx_sub <- idx[has_range] + # Remove indices in ignore + idx_sub <- idx_sub[! idx_sub %in% ignore] # Create new index letters to represent those ranges in the for loops idx_letters <- lapply(1:length(idx_sub), function(i) modelInfo$indexCreator()) idx_letters <- lapply(idx_letters, as.name) @@ -174,7 +186,7 @@ removeIndexAdjustments <- function(idx){ # Check if index range on both sides of declaration are the same # Specifically, all indices on RHS have to be present in LHS # ignore must be a list of quoted code e.g. list(quote(1:n)) -hasMatchingIndexRanges <- function (LHS, RHS, ignore = NULL){ +hasMatchingIndexRanges <- function (LHS, RHS, ignore = list()){ idx_LHS <- extractIndices(LHS) idx_LHS <- idx_LHS[isIndexRange(idx_LHS)] idx_LHS <- lapply(idx_LHS, removeIndexAdjustments) @@ -233,13 +245,14 @@ recursiveReplaceIndex <- function(code, old_idx, new_idx){ # Replace all indices on both sides LHS/RHS # ignore, if provided, must be a list of quoted code # e.g. list(quote(1:n), quote(1:3)) -replaceDeclarationIndexRanges <- function(code, new_idx_list, ignore = NULL){ +replaceDeclarationIndexRanges <- function(code, new_idx_list, ignore = list()){ LHS <- getLHS(code) RHS <- getRHS(code) op <- code[[1]] if(!hasMatchingIndexRanges(LHS, RHS, ignore)) stop("Index ranges must match") idx <- extractAllIndices(LHS) idx <- idx[isIndexRange(idx)] + idx <- idx[! idx %in% ignore] if(length(idx) == 0) return(code) nidx <- length(idx) for (i in 1:nidx){ From 55c61f9c669b8f63bceaf522f40e9ba384363f6e Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Wed, 10 Jun 2026 10:06:28 -0400 Subject: [PATCH 4/7] Add tests --- DESCRIPTION | 4 +- tests/testthat/test_FORLOOP.R | 84 ++++++++++++++++++++++++++ tests/testthat/test_simplifyForLoops.R | 28 +++++++++ 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index bd481c8..495c4ea 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: nimbleMacros -Version: 0.1.1.9003 -Date: 2026-06-09 +Version: 0.1.1.9004 +Date: 2026-06-10 Title: Macros Generating 'nimble' Code Authors@R: c(person("Ken", "Kellner", email="contact@kenkellner.com", role=c("cre","aut")), diff --git a/tests/testthat/test_FORLOOP.R b/tests/testthat/test_FORLOOP.R index 376461a..77b4195 100644 --- a/tests/testthat/test_FORLOOP.R +++ b/tests/testthat/test_FORLOOP.R @@ -322,3 +322,87 @@ test_that("FORLOOP", { nimbleOptions(enableMacroComments=comments_on) }) + +test_that("ignore argument works", { + comments_on <- nimbleOptions()$enableMacroComments + nimbleOptions(enableMacroComments=FALSE) + + # Check error when index on RHS not on LHS, and not ignored + const <- list(y=rnorm(2), x=matrix(rnorm(6), 2, 3), n=2) + modInfo <- list(constants = const, indexCreator = nimble:::labelFunctionCreator("i")) + code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]))) + expect_error(FORLOOP$process(code, modelInfo=modInfo, .env=environment()), + "Index ranges must match") + + # Check malformed ignore arguments + code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignore=1:3)) + expect_error(FORLOOP$process(code, modelInfo=modInfo, .env=environment())$code, + "ignore argument must be a list") + + code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignore=list(1:3, 2))) + expect_error(FORLOOP$process(code, modelInfo=modInfo, .env=environment())$code, + "Some elements of ignore are not quoted code/calls") + + # Simple loop with one ignored range + code <- nimbleCode({ + y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignore = list(quote(1:3))) + }) + mod <- nimbleModel(code, constants=const) + expect_equal( + mod$getCode(), + quote({ + for (i_1 in 1:n) { + y[i_1] <- sum(x[i_1, 1:3]) + } + }) + ) + + # Nested loops + const <- list(y=matrix(rnorm(4), 2, 2), x=array(rnorm(12), c(2,2,3)), n=2) + code <- nimbleCode({ + y[1:n, 1:2] <- FORLOOP(sum(x[1:n, 1:2, 1:3]), ignore = list(quote(1:3))) + }) + mod <- nimbleModel(code, constants=const) + expect_equal( + mod$getCode(), + quote({ + for (i_1 in 1:n) { + for (i_2 in 1:2) { + y[i_1, i_2] <- sum(x[i_1, i_2, 1:3]) + } + } + }) + ) + + # Multiple ignores + const <- list(y=rnorm(2), x=array(rnorm(12), c(2,2,3)), n=2) + code <- nimbleCode({ + y[1:2] <- FORLOOP(sum(x[1:n, 1:2, 1:3]), ignore = list(quote(1:3), quote(1:n))) + }) + mod <- nimbleModel(code, constants=const) + expect_equal( + mod$getCode(), + quote({ + for (i_1 in 1:2) { + y[i_1] <- sum(x[1:n, i_1, 1:3]) + } + }) + ) + + # Ignore on LHS + const <- list(y=matrix(rnorm(4), 2, 2), x=matrix(rnorm(4), 2, 2), n=2) + code <- nimbleCode({ + y[1:n, 1:2] <- FORLOOP(x[1:n, 1:2], ignore = list(quote(1:n))) + }) + mod <- nimbleModel(code, constants=const) + expect_equal( + mod$getCode(), + quote({ + for (i_1 in 1:2){ + y[1:n, i_1] <- x[1:n, i_1] + } + }) + ) + + nimbleOptions(enableMacroComments=comments_on) +}) diff --git a/tests/testthat/test_simplifyForLoops.R b/tests/testthat/test_simplifyForLoops.R index e721875..25bf75e 100644 --- a/tests/testthat/test_simplifyForLoops.R +++ b/tests/testthat/test_simplifyForLoops.R @@ -152,3 +152,31 @@ test_that("Blank index slot is ignored", { ) }) +test_that("Remaining index ranges are ignored", { + # Index ranges could remain in loop code based on use of ignore argument + # make sure simplifyForLoops doesn't replace these with a new index + test <- nimbleCode({ + for (i_1 in 1:n){ + for (i_2 in 1:2){ + y[i_1,i_2] <- sum(x[i_1,i_2,1:m]) + } + } + + for(i_1 in 1:n){ + z[i_1] <- sum(a[1:3]) + } + }) + + out <- simplifyForLoops(test) + + expect_equal(out, + quote({ + for (i in 1:n) { + for (j in 1:2) { + y[i, j] <- sum(x[i, j, 1:m]) + } + z[i] <- sum(a[1:3]) + } + }) + ) +}) From d6871c411c82c459b2afb1713dd4f80b0cbab920 Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Wed, 10 Jun 2026 10:10:08 -0400 Subject: [PATCH 5/7] Set environment when evaluating ignore argument --- R/FORLOOP.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/FORLOOP.R b/R/FORLOOP.R index 3a8f372..0e8fe10 100644 --- a/R/FORLOOP.R +++ b/R/FORLOOP.R @@ -31,7 +31,7 @@ FORLOOP <- nimble::buildMacro( function(code, modelInfo, ignore = list(), .env){ # Super clunky way of extracting the ignore argument # seems to be necessary to do it this way given buildMacro options - ignore <- eval(code[[length(code)]]$ignore) + ignore <- eval(code[[length(code)]]$ignore, envir = .env) # necessary for when ignore is not explicitly included in arguments if(is.null(ignore)) ignore <- list() # get rid of any ignore stuff in code From 4dd2ca404b6cea807314b18d0f4fac9a16bd1140 Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Wed, 10 Jun 2026 10:12:43 -0400 Subject: [PATCH 6/7] Update docs --- DESCRIPTION | 2 +- man/FORLOOP.Rd | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 495c4ea..774a31a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,4 +23,4 @@ URL: https://r-nimble.org BugReports: https://github.com/nimble-dev/nimbleMacros/issues Encoding: UTF-8 VignetteBuilder: knitr -RoxygenNote: 7.3.3 +Config/roxygen2/version: 8.0.0 diff --git a/man/FORLOOP.Rd b/man/FORLOOP.Rd index 52b49fe..53f8a66 100644 --- a/man/FORLOOP.Rd +++ b/man/FORLOOP.Rd @@ -8,6 +8,8 @@ \item{modelInfo}{Used internally by nimbleMacros; a list of model information such as constants and dimensions} +\item{ignore}{Index ranges to ignore when creating for loops; must be provided as a list of quoted code} + \item{.env}{Used internally by nimbleMacros; the environment where the model was created} } \value{ From e5972be8ac711e33e6fc42e204d1cc46b1aab73d Mon Sep 17 00:00:00 2001 From: Ken Kellner Date: Wed, 10 Jun 2026 10:28:35 -0400 Subject: [PATCH 7/7] More informative argument name --- R/FORLOOP.R | 26 +++++++++++++------------- man/FORLOOP.Rd | 2 +- tests/testthat/test_FORLOOP.R | 18 +++++++++--------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/R/FORLOOP.R b/R/FORLOOP.R index 0e8fe10..6045bcb 100644 --- a/R/FORLOOP.R +++ b/R/FORLOOP.R @@ -11,7 +11,7 @@ #' #' @param code The right-hand side of a parameter declaration #' @param modelInfo Used internally by nimbleMacros; a list of model information such as constants and dimensions -#' @param ignore Index ranges to ignore when creating for loops; must be provided as a list of quoted code +#' @param ignoreIndexRanges Index ranges to ignore when creating for loops; must be provided as a list of quoted code #' @param .env Used internally by nimbleMacros; the environment where the model was created #' #' @return NIMBLE code for a for loop or series of nested for loops. @@ -28,19 +28,19 @@ NULL #' @export FORLOOP <- nimble::buildMacro( -function(code, modelInfo, ignore = list(), .env){ +function(code, modelInfo, ignoreIndexRanges = list(), .env){ # Super clunky way of extracting the ignore argument # seems to be necessary to do it this way given buildMacro options - ignore <- eval(code[[length(code)]]$ignore, envir = .env) + ignore <- eval(code[[length(code)]]$ignoreIndexRanges, envir = .env) # necessary for when ignore is not explicitly included in arguments if(is.null(ignore)) ignore <- list() # get rid of any ignore stuff in code - code[[length(code)]]$ignore <- NULL + code[[length(code)]]$ignoreIndexRanges <- NULL # Check ignore structure - if(!is.list(ignore)) stop("ignore argument must be a list", call.=FALSE) + if(!is.list(ignore)) stop("ignoreIndexRanges argument must be a list", call.=FALSE) if(length(ignore) > 0){ is_call <- sapply(ignore, is.call) - if(!all(is_call)) stop("Some elements of ignore are not quoted code/calls", call.=FALSE) + if(!all(is_call)) stop("Some elements of ignoreIndexRanges are not quoted code/calls", call.=FALSE) } # Remove FORLOOP from the code line @@ -185,15 +185,15 @@ removeIndexAdjustments <- function(idx){ # Check if index range on both sides of declaration are the same # Specifically, all indices on RHS have to be present in LHS -# ignore must be a list of quoted code e.g. list(quote(1:n)) -hasMatchingIndexRanges <- function (LHS, RHS, ignore = list()){ +# ignoreIndexRanges must be a list of quoted code e.g. list(quote(1:n)) +hasMatchingIndexRanges <- function (LHS, RHS, ignoreIndexRanges = list()){ idx_LHS <- extractIndices(LHS) idx_LHS <- idx_LHS[isIndexRange(idx_LHS)] idx_LHS <- lapply(idx_LHS, removeIndexAdjustments) idx_RHS <- extractAllIndices(RHS) idx_RHS <- idx_RHS[isIndexRange(idx_RHS)] idx_RHS <- lapply(idx_RHS, removeIndexAdjustments) - idx_RHS <- idx_RHS[!idx_RHS %in% ignore] + idx_RHS <- idx_RHS[!idx_RHS %in% ignoreIndexRanges] if(length(idx_RHS) == 0) return(TRUE) all(idx_RHS %in% idx_LHS) } @@ -243,16 +243,16 @@ recursiveReplaceIndex <- function(code, old_idx, new_idx){ } # Replace all indices on both sides LHS/RHS -# ignore, if provided, must be a list of quoted code +# ignoreIndexRanges, if provided, must be a list of quoted code # e.g. list(quote(1:n), quote(1:3)) -replaceDeclarationIndexRanges <- function(code, new_idx_list, ignore = list()){ +replaceDeclarationIndexRanges <- function(code, new_idx_list, ignoreIndexRanges = list()){ LHS <- getLHS(code) RHS <- getRHS(code) op <- code[[1]] - if(!hasMatchingIndexRanges(LHS, RHS, ignore)) stop("Index ranges must match") + if(!hasMatchingIndexRanges(LHS, RHS, ignoreIndexRanges)) stop("Index ranges must match") idx <- extractAllIndices(LHS) idx <- idx[isIndexRange(idx)] - idx <- idx[! idx %in% ignore] + idx <- idx[! idx %in% ignoreIndexRanges] if(length(idx) == 0) return(code) nidx <- length(idx) for (i in 1:nidx){ diff --git a/man/FORLOOP.Rd b/man/FORLOOP.Rd index 53f8a66..465ac64 100644 --- a/man/FORLOOP.Rd +++ b/man/FORLOOP.Rd @@ -8,7 +8,7 @@ \item{modelInfo}{Used internally by nimbleMacros; a list of model information such as constants and dimensions} -\item{ignore}{Index ranges to ignore when creating for loops; must be provided as a list of quoted code} +\item{ignoreIndexRanges}{Index ranges to ignore when creating for loops; must be provided as a list of quoted code} \item{.env}{Used internally by nimbleMacros; the environment where the model was created} } diff --git a/tests/testthat/test_FORLOOP.R b/tests/testthat/test_FORLOOP.R index 77b4195..d7bcba6 100644 --- a/tests/testthat/test_FORLOOP.R +++ b/tests/testthat/test_FORLOOP.R @@ -323,7 +323,7 @@ test_that("FORLOOP", { nimbleOptions(enableMacroComments=comments_on) }) -test_that("ignore argument works", { +test_that("ignoreIndexRanges argument works", { comments_on <- nimbleOptions()$enableMacroComments nimbleOptions(enableMacroComments=FALSE) @@ -335,17 +335,17 @@ test_that("ignore argument works", { "Index ranges must match") # Check malformed ignore arguments - code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignore=1:3)) + code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignoreIndexRanges=1:3)) expect_error(FORLOOP$process(code, modelInfo=modInfo, .env=environment())$code, - "ignore argument must be a list") + "ignoreIndexRanges argument must be a list") - code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignore=list(1:3, 2))) + code <- quote(y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignoreIndexRanges=list(1:3, 2))) expect_error(FORLOOP$process(code, modelInfo=modInfo, .env=environment())$code, - "Some elements of ignore are not quoted code/calls") + "Some elements of ignoreIndexRanges are not quoted code/calls") # Simple loop with one ignored range code <- nimbleCode({ - y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignore = list(quote(1:3))) + y[1:n] <- FORLOOP(sum(x[1:n, 1:3]), ignoreIndexRanges = list(quote(1:3))) }) mod <- nimbleModel(code, constants=const) expect_equal( @@ -360,7 +360,7 @@ test_that("ignore argument works", { # Nested loops const <- list(y=matrix(rnorm(4), 2, 2), x=array(rnorm(12), c(2,2,3)), n=2) code <- nimbleCode({ - y[1:n, 1:2] <- FORLOOP(sum(x[1:n, 1:2, 1:3]), ignore = list(quote(1:3))) + y[1:n, 1:2] <- FORLOOP(sum(x[1:n, 1:2, 1:3]), ignoreIndexRanges = list(quote(1:3))) }) mod <- nimbleModel(code, constants=const) expect_equal( @@ -377,7 +377,7 @@ test_that("ignore argument works", { # Multiple ignores const <- list(y=rnorm(2), x=array(rnorm(12), c(2,2,3)), n=2) code <- nimbleCode({ - y[1:2] <- FORLOOP(sum(x[1:n, 1:2, 1:3]), ignore = list(quote(1:3), quote(1:n))) + y[1:2] <- FORLOOP(sum(x[1:n, 1:2, 1:3]), ignoreIndexRanges = list(quote(1:3), quote(1:n))) }) mod <- nimbleModel(code, constants=const) expect_equal( @@ -392,7 +392,7 @@ test_that("ignore argument works", { # Ignore on LHS const <- list(y=matrix(rnorm(4), 2, 2), x=matrix(rnorm(4), 2, 2), n=2) code <- nimbleCode({ - y[1:n, 1:2] <- FORLOOP(x[1:n, 1:2], ignore = list(quote(1:n))) + y[1:n, 1:2] <- FORLOOP(x[1:n, 1:2], ignoreIndexRanges = list(quote(1:n))) }) mod <- nimbleModel(code, constants=const) expect_equal(