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

TTR::EMA and TTR::DEMA Profiling #69

Closed
ckatsulis opened this issue Jun 21, 2018 · 3 comments
Closed

TTR::EMA and TTR::DEMA Profiling #69

ckatsulis opened this issue Jun 21, 2018 · 3 comments
Milestone

Comments

@ckatsulis
Copy link

ckatsulis commented Jun 21, 2018

On a quick trial of supplying data to TTR as an xts object and then as coredata and recasting as xts, I was able to achieve performance improvements on the order of 60%. Are you able to explain why this is the case, or could you potential modify the call so that it does this naturally?

> dim(testData)
[1] 5783631       1

Pass as coredata and recast as xts with .xts and .index:

.xts(TTR::DEMA(coredata(testData)),.index(testData))

1220ms
image

Pass as original xts object:

profvis::profvis(TTR::DEMA(testData))

2760ms
image

@joshuaulrich
Copy link
Owner

Thanks for the report! The xts-input version is slower because the reclass() call in EMA() converts the result of .Call("ema", x, n, ratio) from a vector to a xts object (a matrix, with a dim attribute). That causes column names to be added, which creates copies of intermediate objects and causes the slowness.

One potential solution is to create a dema C function that calls the ema C function. That would require adding try.xts() and reclass() to DEMA(), since it would no longer get those automatically from EMA().

joshuaulrich added a commit that referenced this issue Jun 23, 2018
This makes it easier to call the ema() C function from other C
functions, which can help avoid some of the overhead in R function
calls.

See #69.
joshuaulrich added a commit that referenced this issue Jun 23, 2018
Similar to the previous commit, this makes it easier to call ema()
from other C functions.

See #69.
@joshuaulrich
Copy link
Owner

@ckatsulis The changes on this branch help close the performance gap in my tests. There's also an xts performance branch that contains some other relevant changes to help make this a bit faster.

The DEMA(x) call is still slower because the try.xts() and reclass() calls cause more temporary copies of objects, which usually means more garbage collections.

I would really appreciate if you could update both packages and provide feedback!

Here's the data and tests I was running:

require(xts)
require(TTR)
require(microbenchmark)
x <- .xts(rnorm(1e7), 1:1e7)

xts.dema <- function(x) {
  .xts(DEMA(coredata(x)), .index(x))
}
ttr.dema <- function(x) {
  DEMA(x)
}

Rprof(); o1 <- xts.dema(x); Rprof(NULL); summaryRprof()
Rprof(); o2 <- ttr.dema(x); Rprof(NULL); summaryRprof()
all.equal(drop(coredata(o1)), drop(coredata(o2)))

microbenchmark(xts.dema(x), ttr.dema(x), times = 5)
# Unit: milliseconds
#         expr      min       lq     mean   median       uq      max neval cld
#  xts.dema(x) 386.1508 413.3212 416.1767 425.5015 426.4050 429.5053     5  a 
#  ttr.dema(x) 456.2319 487.6094 493.3008 495.4675 509.4217 517.7736     5   b

@joshuaulrich joshuaulrich added this to the v0.23-4 milestone Sep 9, 2018
@ckatsulis
Copy link
Author

ckatsulis commented Sep 17, 2018 via email

joshuaulrich added a commit that referenced this issue Aug 16, 2020
This continues the work in f902a19,
which makes it easier and safer to call ema() from other C functions.

See #69.
joshuaulrich added a commit that referenced this issue Aug 30, 2020
This uses many of the same checks, in the same order, as the ema() C
function. That makes it easier and safer to call zlema() from other C
functions.

This also fixes the issue flagged by clang-UBSAN: We approximated 'n'
using 'ratio', but it was possible that ratio = 0. There was a check
for ratio > 0 later in the function, but that didn't prevent the
possibility of division by 0.

Thanks to Prof Ripley for the report.

See #100, see #69.
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

2 participants