Nuts divergences fix

[danielturek]

NUTS sampler correctly tracking numDivergences, the number of divergent paths encountered.

[danielturek]

WIP, do not merge.

Fixes #81

[danielturek]

@paciorek Do you know what's the problem with the (Windows only) testing failures here? The failure message is copied below:

Run RCMD INSTALL --install-tests nimbleHMC
* installing to library 'D:/a/_temp/Library'
* installing *source* package 'nimbleHMC' ...
** this is package 'nimbleHMC' version '0.2.5'
** using staged installation
** R
** inst
** tests
** byte-compile and prepare package for lazy loading
Error: Error: package or namespace load failed for 'nimble' in inDL(x, as.logical(local), as.logical(now), ...):
 unable to load shared object 'D:/a/_temp/Library/rlang/libs/x64/rlang.dll':
  LoadLibrary failure:  The specified procedure could not be found.
Execution halted
ERROR: lazy loading failed for package 'nimbleHMC'
* removing 'D:/a/_temp/Library/nimbleHMC'
Error: Process completed with exit code 1.

[paciorek]

It might be this change to R.

I think it might have something to do with an older binary for rlang being installed in the testing environment.

I can look more this coming week.

[danielturek]

Thanks, Chris. I appreciate any help.

[paciorek]

Updating the Windows runner to R 4.6 fixed that issue. Presumably because the rlang binary was not compatible with the older Windows, though I haven't grasped all the details.

There is now a tolerance-related test failure on MacOS.

[danielturek]

@paciorek Can you please explain a bit more what you meant by "Updating the Windows runner to R 4.6 fixed that issue" ?

I don't see any changes or commits that you might have made in this PR, and the two Windows test-related errors still appear to be present.

[paciorek]

Whoops, I made the change in commit 12e0f7c on master, not here...

Can you fix things up?

[danielturek]

@paciorek Thank you!

[danielturek]

@paciorek The reason I always had Imports: methods in the DESCRIPTION file is that (locally) running devtools::check("nimbleHMC") results in the following:

   
── R CMD check results ───────── nimbleHMC 0.2.5 ────
Duration: 2m 16.2s

❯ checking dependencies in R code ... NOTE
  package 'methods' is used but not declared

0 errors ✔ | 0 warnings ✔ | 1 note ✖

That said, I'm happy to try submitting to CRAN is it currently is (without Imports: methods in the DESCRIPTION file), and seeing what CRAN tests indicate.

[paciorek]

I think it's generally a bad idea to submit with a Note like this being present.

So I think we either need import(methods) in NAMESPACE or if we can isolate a small number of uses of it in our code, to use importFrom(methods,foo).

[danielturek]

I can confirm that adding import(methods) to NAMESPACE does not prevent this warning.

However, (as previous) when I had Imports: methods in the DESCRIPTION, this warning does not appear - but that seemed to cause the reverse dependency problem that you mentioned for nimble package (#45).

I'm at a loss for where any functions from methods package are used in the nimbleHMC source code.

[paciorek]

That's weird -- if I add import(methods) to NAMESPACE, then R CMD check --as-cran on Linux does prevent the warning (which is what I would expect).

We could send nimbleHMC to winbuilder and see if the warning shows up there if you do make that addition to NAMESPACE.

[danielturek]

In order to avoid these warnings (using R CMD check --as-cran) I now have imports(methods) in the NAMESPACE, and Depends: methods in the DESCRIPTION. That said, @paciorek has indicated that having these causes a problem with the reverse dependencies check from nimble, so I'm not sure which course is correct.

[paciorek]

Actually, I think the rev dep check was just flagging the case that there was no use of method in NAMESPACE and was consistent with the results of simply running R CMD check on nimbleHMC directly. That's all the rev dep check does anyway.

[danielturek]

@paciorek Ok, thanks for the explanation. I'm happy with this PR, and if @paciorek or @perrydv want to take a look before merging, that would be great.