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

Adding arima.c, make build work #233

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tninja
Copy link

@tninja tninja commented Aug 14, 2016

The arima.c get copied here, it include ARIMA_transPars function. C_ARIMA_transPars is defined in renjin.R. The build is working now, but for some reason the arima() report ERROR: Could not resolve native method ARIMA_transPars.

@akbertram, do you have any clue what I missed? Thanks in advance.

@bddbot
Copy link

bddbot commented Aug 14, 2016

Can one of the admins verify this patch?

@akbertram
Copy link
Member

@bddbot add to whitelist

@akbertram
Copy link
Member

Looks good!

I think you are probably missing an entry in the stat package's NAMESPACE file.

Can you also add a simple test case for the arima function? See test.stats.runmed.R for a simple example.

Thanks again!

@@ -41,7 +41,11 @@ SEXP nextn(SEXP n, SEXP factors);

SEXP cfilter(SEXP sx, SEXP sfilter, SEXP ssides, SEXP scircular);
SEXP rfilter(SEXP x, SEXP filter, SEXP out);

#ifndef R_STATS_H
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to avoid build conflict: in stats.h there is lowess, and arima.c include both stats.h and statsR.h.

@tninja
Copy link
Author

tninja commented Aug 14, 2016

Not quite easy..

After adding stuffs inside arima.c to NAMESPACE, now it report:

arima(lh, order = c(3,0,0), optim.method="Nelder-Mead")
ERROR: Sorry! polyroot not yet implemented!
java.lang.ClassNotFoundException: org.renjin.primitives.R$primitive$polyroot

Found a do_polyroot function inside complex.c of R source code:
SEXP attribute_hidden do_polyroot(SEXP call, SEXP op, SEXP args, SEXP rho)

Not sure how to get this thing work, I don't even know where to put complex.c. Do you have any clue?

@akbertram
Copy link
Member

Yes, this is a bit trickier. polyroot is an internal base function that basically has two parts:

  1. R_cpolyroot and its subroutines constitute the actual C implementation of the algorithm and is largely independent of any R types or methods. It can be added to the math/nmath module or perhaps we can create a new math/complex module.

  2. The do_polyroot() is the R wrapper for R_cpolyroot. Because it's in the base package, it's difficult to compile the C code directly, so I'd suggest just porting it to Java. In Renjin, internal functions are implemented as static methods and registered in the Primitives class. See the complex function as an example.

ARIMA_Like,
ARIMA_CSS,
TSconv,
getQ0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all of these functions actually exported and used in the R code, or are some of them internal to the C code? Check the stat's package NAMESPACE file in GNU R:
https://github.com/wch/r-source/blob/trunk/src/library/stats/NAMESPACE

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

Successfully merging this pull request may close these issues.

3 participants