Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error trap for reduction operations on vector arguments #1525

Open
rperry-usgs opened this issue Jan 23, 2025 · 1 comment
Open

Error trap for reduction operations on vector arguments #1525

rperry-usgs opened this issue Jan 23, 2025 · 1 comment

Comments

@rperry-usgs
Copy link

Hi there! I ran into this problem, fixed it, and subsequently read that nimble functions don't handle negative indices, as in R. Indeed, in a function more complex than the reproduceable example below (a custom sampler), I received an error when indexing by x[-d] where d = length(x). Here's the specific error returned on compilation:

Error:
Error: NIMBLE compiler does not support reduction operations on scalar arguments.
This occurred for: sum(x_next[-(d)])
This was part of the call: totTime - sum(x_next[-(d)])

However, the function compiled successfully and happily returned a wrong result when using x[-c(i,d)] where i is a for loop index. I only caught this error because results did not match my sampler implemented in R, but I could see how it might go unnoticed or be difficult to debug in some cases. So I'm flagging this as a potential candidate for a stop-execution error trap that doesn't return a result, similar to the error message above.

Below is a simple case reproducing the error. I'm using nimble v1.3.0.

testNegIdx <- nimbleFunction(
run = function(x = double(1)) {
ans <- x[-c(1,4)]
return(ans)
returnType(double(1))
} )

Should return c(2, 3), and does.
x <- 1:4

testNegIdx(x)
[1] 2 3

Test compiled version
`CtestNegIdx <- compileNimble(testNegIdx)

CtestNegIdx(x)
[1] 7.410985e-323 8.332910e-312
CtestNegIdx(x)
[1] 0.000000e+00 8.332191e-312
CtestNegIdx(x)
[1] 1.022650e-312 1.447621e+166

Not c(2,3) and returns different values each time it's called.

@paciorek
Copy link
Contributor

@rperry-usgs thanks for this report.

Indeed in both the x[-2] and x[-c(2,4)] cases we should error trap. Furthermore, it doesn't look to me that we mention this lack of functionality in the manual.

Let's consider putting a check for - in sizeIndexingBracket. It looks like we could simply check for - in code$args, e.g,

sapply(code$args, \(x) exists('name', x) && x$name =="-")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants