reorder final lines of NUTS run

[perrydv]

In sampler_NUTS, this PR moves the final calls to calculate and nimCopy (that leave the model fully up-to-date upon exit) to be the last lines, appearing after the adaptation calls rather than before. This avoids the problem that the adaptation calls, when initEpsilon is called, use the model and hence can change its state.

[danielturek]

@perrydv Two comments:

1. It seems plausible that inverseTransformStoreCalculate(state_sample$q) should occur before the adaptation also, in particular in the case when initEpsilon will be called. That's because the routines taking place in initEpsilon depend on the model values, so these should be up-to-date.

So, if I'm seeing it correctly, the updated code could look something like:

if((timesRan <= nwarmup) & adaptive) {
    if(adaptEpsilon)   adapt_stepsize(accept_prob)
    update <- FALSE
    if(adaptM)   update <- adapt_M()
    if(update & adaptEpsilon) {
        if(initializeEpsilon) {
            inverseTransformStoreCalculate(state_sample$q)     ## only called in advance, in initEpsilon case
            initEpsilon()
        }
        Hbar <<- 0
        logEpsilonBar <<- 0
        stepsizeCounter <<- 0
        mu <<- log(10*epsilon)
    }
}
inverseTransformStoreCalculate(state_sample$q)      ## always called here
nimCopy(from = model, to = mvSaved, row = 1, nodes = calcNodes, logProb = TRUE)    ## only place copy is called

Thoughts @perrydv ?

2. It seems similar changes should also be made for the NUTS_classic sampler. Right?

[perrydv]

@danielturek I looked at NUTS_classic and didn't think the same change is needed because the adaptiveProcedure does not use the model. However it would seem harmless to make the same change and thus remove any question when looking at the code in the future.

[danielturek]

@perrydv What do you think of the updated code, in my earlier comment from several days ago? The idea was to also set the model state correctly, in advance of calling the initEpsilon method. Does that make sense from your perspective?

[perrydv]

@danielturek I think your suggestion makes sense. I think it is defensive in that I think the model should be up to date, but initEpsilon is not called many times and so the number of additional model calculations needed for this defensiveness should not be a noticeable cost. I also realized I was wrong about the NUTS_classic and it does also need similar changes. I have made those changes and pushed them on this branch without testing locally, so let's see if tests run and please review the code.

[danielturek]

@perrydv Agreed on all counts. I reviewed the changes you made, and LGTM.