search

nimble-dev / nimbleEcology

18 stars 9 forks source link

AD support #27

Closed perrydv closed 5 months ago

perrydv commented 5 months ago

Here is the PR with AD support for the nimbleEcology distributions. This includes:

weizhangstats commented 5 months ago

@perrydv Just to let you know that all N-mixture models with AD supported are in the Nmixture-AD branch. You can see how to use the code there. Let me know you want me to do anything.

dochvam commented 5 months ago

@perrydv Looks like dNmixture's AD test is failing during a NIMBLE compilation step if you want to take a look. Is this related to Wei's comment about the two different branches?

perrydv commented 5 months ago

@weizhangstats Thanks for the comment. Yes, sorry for not explaining. For the reasons commented in your PR, I wanted to work on the Nmixture AD support a bit more. I wanted to retain a way for non-AD users to get the automatic Nmin and Nmax determination, and I wanted to separate the numerical stability "max_index" from getting baked in arbitrarily on the first call. (It is still baked in, but with a heuristic choice rather than arbitrarily due to parameters involved in the first call). It was easiest for me to consolidate everything in this PR with the other distributions, having drawn on your branch where I could.

@dochvam I will look at the test failure. I think you fixed the yaml setup, thanks. The tests run on my machine, so I'm hoping it is some trivial issue to fix.

perrydv commented 5 months ago

I don't know the actions and yaml setup very well. I noticed it was still installing nimble branch ADoak, but at this point it should use the released version of nimble, so I changed that. I also updated some tests.

I also realized that the dBetaBinom naming seemed inconsistent with the others, so I created dBetaBinom_v and dBetaBinom_s. There could also be a dBetaBinom_oneObs, but it would not be needed by the Nmixture cases so I didn't create it.

perrydv commented 5 months ago

Status update: Various details are tweaked and fixed and tests pass on OS X and Windows. Tests are not passing on Linux and this appears to be due to the testing setup. I tried tweaking the linux-specific bits of the yaml by imitating nimble's, but I wasn't sure of some details. R CMD check --as-cran passes except for warnings about the html related to the .Rd files. This seems related to a discussion on r-sig-mac, and it looks like those warnings can be ignored.

So I think it is very nearly ready to go. It would be nice to get linux testing working. And it would be good to have review of code and documentation.

paciorek commented 5 months ago

I'm no longer seeing the Linux tests in the GHA report.

dochvam commented 5 months ago

I do see the GHA checks on ubuntu in the report, looks like they're all passing!

paciorek commented 5 months ago

Oh, I see. There are two separate actions, one for windows and one not, so one needs to toggle between them via links on the upper left, unlike with the GHA for nimble itself.