Skip to content

add handling of (expression)[indexing] in addMissingIndexingRecurse#929

Merged
paciorek merged 8 commits into
develfrom
fix_nct_141
Oct 30, 2019
Merged

add handling of (expression)[indexing] in addMissingIndexingRecurse#929
paciorek merged 8 commits into
develfrom
fix_nct_141

Conversation

@paciorek
Copy link
Copy Markdown
Contributor

FRO-DNM

This looks for cases like (x[1:2]%*%y[1:2])[1,1] based on ( and [ and if found, checks no missing indices and then runs addMissingIndexingRecurse on the parenthesized expression being indexed.

Before finalizing this I would also like to resolve the question raised in NCT issue 141 regarding whether the stanza beginning line 562 of BUGS_modelDef.R that checks for $ is needed.

Will need to add testing, including testing for correct trapping of missing indexing and vector version of this situation, namely something like (y[1:2]*x[1:2])[2]

@paciorek paciorek added the bug label Jul 29, 2019
@paciorek paciorek requested review from danielturek and perrydv July 29, 2019 00:46
@paciorek paciorek self-assigned this Jul 29, 2019
of expressions in BUGS code, for now
@paciorek
Copy link
Copy Markdown
Contributor Author

One further note here - because of checking I added, this PR errors out for missing indexes of expressions, e.g.,

code = nimbleCode({
    mn[1:2] <- (X[1:2,1:2] %*% beta[1:2,1:2])[,1]
    y[1:2] ~ dmnorm( mn[1:2], pr[1:2,1:2])
})
m = nimbleModel(code, data = list(y = rnorm(2)),
                inits = list(X = matrix(1, 2,2), beta = matrix(2,2,2), pr = diag(2)))
cm <- compileNimble(m)
cm$calculate('y')

However, if I take that check out, compilation goes through and calculations are fine. So we may well not want to check for missing indexes, unless we think there are cases where missing indexing will cause problems. Need input from @perrydv on what happens in terms of translation to Eigen code when indexes are missing.

add recursion into the indexing elements
@paciorek
Copy link
Copy Markdown
Contributor Author

  • Fixed erroneous commenting of the check for missing indexes in ()[,].
  • Recurse into the indexes of [,] when have things like k[,1]: [k[,1],]
  • Added testing

Waiting on Travis.

monkey with tests for indexing given flightiness in message vs. output vs silent
@paciorek
Copy link
Copy Markdown
Contributor Author

Apparently here's the use case for needing to check $ (see line 1284 of test-nimbleLIst.R)

    modelCode1 <- nimbleCode({
        meanVec[1:5] <- eigen(constMat[,])$values[1:5]
        covMat[1:5,1:5] <- eigen(constMat[1:5,])$vectors[1:5,1:5]%*%t(eigen(constMat[1:5,1:5])$vectors[,])
        y[] ~ dmnorm(meanVec[], cov = covMat[1:5,1:5])
    })

Adding this back in and rerunning tests.

@paciorek paciorek merged commit b8216f2 into devel Oct 30, 2019
@paciorek paciorek deleted the fix_nct_141 branch October 30, 2019 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant