Turn into an installable R package

[lentinj]

@bthe we talked at some point about turning this repo into an R package so the gadget executable can be installed via. install_github().

Not only would this be useful for rgadget users, it'd make an automated build of the gadget course documentation (and probably Rgadget) a lot easier, since we wouldn't have to have a separate build step to install Gadget before doing whatever we were going to do.

I'm assuming you'd like me to have a go at some point and see how easy it is?

[bthe]

Sounds good to me:)

[lentinj]

Okay, there's a branch for this: https://github.com/Hafro/gadget/tree/r-package - I've done a bunch of restructuring so R and the top-level make share most of the compilation. There's also a gadget_binary() for finding where gadget is (this seems a bit convoluted, but if I find out the right way of doing this can easily swap it out).

Remaining problems:

• Probably doesn't work under windows, should check.
• At least locally, R's makeconf is enabling -g on anything compiled, and makes the debug output 10x as big. Might be a non-issue, as the debug output could be useful, but it causes CMD check to bleat.
• Document installation via. install_github() (after testing it works :P) in the README.
• src/Makefile has GNU-isms. I'm not sure it's worth caring, but may cause a problem on MacOS.

[bthe]

Trying to compile on windows gives me a weird error stating that it can't find areatime.h, which I guess means that the compiler does not find the headers folder.

[lentinj]

Yup, I get that too. The -D flags are missing too. Will continue to tinker.

[lentinj]

Right! That was annoying. But the branch should work under windows now: remotes::install_github('Hafro/gadget', 'r-package').

Think we can merge it to master.

I've added some logic to callGadget to use this if available. Not sure if we want to make it depend on the gadget package?

Also on this note whilst gadget and rgadget are free on CRAN, GADGET is not. It seems unlikely they'll allow an entirely unrelated lower-case gadget as well, so it might be worth thinking of a better name before the package name gets on too many computers.

EDIT: cppgadget maybe?

[bthe]

Yes, we may want to think about naming of the series of packages we are/will be creating, and judging by packages on CRAN there seems to be no logic to naming or style, so I would be interested to see if they accept a package named gadget.

But in any case, would a naming scheme like this work:

• gadget.cpp, legacy gadget in c++
• gadget.r, the gadget simulator in R, which is currently a part of Rgadget but should be reworked as standalone package
• gadget.tmb, the updated gadget in TMB

with Rgadget as the umbrella package to interact with other packages?

And yes, by all means merge with the master branch.

[lentinj]

I would be interested to see if they accept a package named gadget.

Unsurprisingly, check --as-cran says no: Conflicting package names (submitted: gadget, existing: GADGET [https://CRAN.R-project.org]). Even if they did, I'm not sure we'd want to impose the mess on ourselves.

would a naming scheme like this work

I'm nervous about the subtle distinction between gadget.r and Rgadget. Is there much value in gadget.r once we have both CPP and TMB-based gadget?

Another option is gadget2 (CPP-based gadget) and gadget3 (TMB-based gadget). It seems unlikely that CPP-based gadget will have another major revision at this point. Whilst a ugly, this does make the intent a bit more obvious.

[bthe]

I like that idea, the gadget simulator in R is not a priority but could be used more for testing purposes when developing TMB gadget.

[lentinj]

Okay, after some train-tinkering this is basically done.

• Can we rename this repository to gadget2? Github will redirect from the old name to new, and will mean that it's more obvious what package you're installing from the install_github() command.
• Are we going to attempt a CRAN upload? If so overrides of GCC optimisation flags will need to be removed. This means we may have to tell people to set up a ~/.R/Makevars that sets -O3 and friends (the apparent-right way of doing things), but probably worth checking it makes a significant difference to runtime first.

[bthe]

• Have no qualms with renaming the repository.
• I would think it would be a good idea to have both gadget2 and Rgadget on CRAN, as it would save me a lot of headaches. I'm currently testing the speed with and without optimizations to see if there are any significant differences wrt speed, I would actually expect that so the makevars route is probably the best for serious gadgeteers whilst the casual (windows) user would probably not care as much.

[bthe]

On the difference between using compiler optimization or not, it appears that there is a difference in runtime in excess of at least 30% so I don't think it is viable for the end user to run gadget without the -O3 switch for longer runs.

[lentinj]

Right, I've modified the Makefile to default to -O3 -march=native in a check-as-cran-compatible way. Given this is compiling a stand-alone binary hopefully it should be allowed.

I need to add a changelog file and a version-bumping script, then we should upload and see what happens, unless there's any problems with the current version of gadget2 that shouldn't be released?

MFDB equally needs releasing before Rgadget, but that's a lot less esoteric as packages go so hopefully won't be a problem.

[bthe]

Only thing I can think of is when I compile the package using clang I get a series of warnings similar to:

initialinputfile.cc:189:7: warning: unused variable 'i' [-Wunused-variable]
  int i;
      ^
1 warning generated.

These appear all to be related on unused loop indices and I'm not sure if this causes any problems on the CRAN servers.

[lentinj]

You don't need clang for that, just compile with -Wall. I've cleaned up all the easy unused variables, and had a stab at rewriting PopStatistics::calcStatistics(const AgeBandMatrix& agelenum, int lengr), which I suspect never worked. ProgLikelihood::CalcTac(const TimeClass* const TimeInfo) also needed some more in-depth sorting.

CRAN tests may look for warning, depending on the configuration. Whether we'd get away with it or not I'm not sure, but best to tidy warnings up anyway.

[bthe]

Cool, should we submit it to CRAN?

[lentinj]

Do you have any tame MacOS users nearby? That's the only other thing I can think of trying.

[bthe]

Yes, me;) Running the newest version of MacOs and the package compiles and runs without a warning, and the output makes as much sense as on linux.

[lentinj]

I've submitted it to win-builder.r-project.org for a final check. If that passes I'll submit it.

[lentinj]

Okay, I've submitted it. You'll probably get the email to confirm as the maintainer.

I called this 2.3.0 - https://github.com/Hafro/gadget2/commit/37a07064357ca78b284542d7093e24e2e0075481 the legwork here is done with ./do-release 2.3.0. Also note that the version in master is now 2.3.99 - this is the best way I know of to say -dev -BETA or generally that this isn't released, since version numbers have to be numeric. ./do-release 2.3.1, if it needs to be made will still work, and ./do-release will go back to 2.3.99 afterwards.

[bthe]

Gadget is now available from CRAN (https://cran.r-project.org/web/packages/gadget2/index.html) so I think it is time to close this issue.