Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-code to omit the use of call_R. #11

Closed
swihart opened this issue Jan 28, 2022 · 0 comments
Closed

Re-code to omit the use of call_R. #11

swihart opened this issue Jan 28, 2022 · 0 comments

Comments

@swihart
Copy link
Owner

swihart commented Jan 28, 2022

Luckily I've hammered this out in gnlrim. See this issue for the stream of consciousness journey of learning and recoding and how I handled the issue in repeated

The biggest difference from the gnlrim and repeated experience: rmutil doesn't do roxygen. So don't do any CMD+SHFT+D stuff.

The second biggest difference is that the call_R() wasn't just in romberg.c. It was also in the entry inthp / TOMS614. They are called in the different int and int2 functions.

So here's how we will tackle it:

  • ✅ Make a new branch.

  • ✅ Change the version in the DESC

  • ✅ Change the NEWS file

  • ✅ Change the cran-comments file

  • rhub::check(platform = "debian-clang-devel") to see warnings about call_R for both romberg and toms

  • ✅ start working on romberg first since we have experience

  • ✅ drop gnlrim/src/romberg_sexp.c into repeated/src/romberg_sexp

  • ✅ edit src/repeated_init.c to accommodate the .Call() entry for romberg_sexp

    • ✅ put #include <Rinternals.h> at the top so it knows SEXP type
    • ✅ use SEXP romberg_sexp(SEXP fcn, SEXP a, SEXP b, SEXP len, SEXP eps, SEXP pts, SEXP max, SEXP err, SEXP envir); instead of extern void romberg_sexp(void *, void *, void *, void *, void *, void *, void *, void *, void *, void *);
  • ✅ edit chunks starting at line 40 and again at line 197 changing .C("romberg" into .Call("romberg_sexp"

    • ✅ (remembering PACKAGE="rmutil")
  • ✅ hold your breath CMD+SHFT+B

  • ✅ close out R studio and delete all *.o files; reopen project

  • ✅ hold your breath CMD+SHFT+B

  • ✅ test a bit on examples.

  • ✅ since you've removed romberg.c from the pipeline you need to edit repeated_init.c -- just comment out the lines pertaining to romberg_centry
    * ✅ delete all *.o, CMD+SHFT+B, and rhub::check(platform = "debian-clang-devel") successful w/out warnings about call_R from romberg (toms is ok, we'll get that in a sec)

  • ✅ do enough rhubs to write in the cran-comments that you tested thoroughly and only getting complaints about toms

  • ✅ git add; git commit

  • 🌟 Now do it again for TOMS614!

  • ✅ Need to look closely at the C. First off I want to see if I can remove the .fsrc file and be ok. so remove it and do a CMD+SHFT+B and a rhub::check(platform = "debian-clang-devel"). Also run examples from ?int using TOMS

  • ✅ edit rmutil_init: comment out .C related lines; write .Call related lines similar to romberg_sexp -- toms614 only has inthp in it.

  • copy toms614.c to toms614_sexp.c. and do the following: BE VERY CAREFUL.

    • put in same header refs as the tops of romberg_sexp.c
    • call_R's are at line 458; 467; 476, 485, 520, 533, 593, 608
    • looking at how length is used it looks like it is set to 1, where as it was a variable in romberg. This observation combined with reading the ?int help file shows that only romberg was vectorized; toms614 was not. So i think this might simplify some of the for loops implementation I was doing -- as in, I won't have to do them. And i can just set the length of the state2 vector to 1. I can protect/unprotect things at the top/bottom of each if() statement. Let's jump in.
    • ✅ comment out lines 40-42; lines 278-280 (anything involving mode, ss, length, args)
    • ✅ comment out line 39; tmp is going to be like x in romberg and we're going to SEXP it up
    • ✅ put SEXP call2, result2, state2, tmp; somewhere, say line 277
    • here's lines 455-461
    if (*inf == 1) {
      /*	sum = (*f)(&c_b12);*/
    zz[0]=c_b12;
    call_R(f, 1L, args, mode, length, 0L, 1L, ss);
    tmp=(double *)ss[0];
    sum=tmp[0];
    }

try replacing every call_r statement with;

    if (*inf == 1) {   
      /*	sum = (*f)(&c_b12);*/
    zz[0]=c_b12;

    state2 = Rf_protect(Rf_allocVector(REALSXP, 1)); /*.C -> .Call requires this*/
    REAL(state2)[0] = zz[0];                                   /*.C -> .Call requires this*/
    Rf_protect(call2=Rf_lang2(f, state2));        /*.C -> .Call requires this*/
    Rf_protect(result2=Rf_eval(call2, envir));     /*.C -> .Call requires this*/
    Rf_protect(tmp=Rf_coerceVector(result2, REALSXP));/*.C -> .Call requires this*/    

    sum=REAL(tmp)[0];
    Rf_unprotect(4);
    }
  • remember to make SEXP f; replace quadr with SEXP envir as last argument
  • declare double quadr;
  • SEXP ans;
  • Rf_protect(ans = Rf_allocVector(REALSXP, *LEN));
  • REAL(ans)[0] = quadr[0];
  • replace return 0; with REAL(ans)[0] = quadr[0]; Rf_unprotect(1); return ans;// return 0;
  • then remember that all the args have to be SEXP and then you have to declare them again as the *capitallized version of the argument, then find/replace all of them
  • and I'll be honest I started hacking on stuff and got ahead of myself and trusted my gut and looks like it is working.

Then

  • ✅ do enough rhubs to write in the cran-comments that you tested thoroughly and only getting complaints about ANYTHING.
  • ✅ git add; git commit
  • ✅ push branch to github
  • ✅ merge with master in browser on github
  • ✅ get back on master branch in Rstudio
  • ✅ git pull
  • ✅ submit to cran with devtools::submit_cran()
  • ✅ check CRAN results to see if worked.
  • ✅ given it worked, celebrate having fixed repeated and rmutil!
  • 🎆 SUPER BONUS! -- consider getting gnlrim to use int2 and make it handle 2 random effects! (in progress).
@swihart swihart closed this as completed Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant