quantmod icon indicating copy to clipboard operation
quantmod copied to clipboard

modelReturn incorrectly calculates days.traded

Open AndreMikulec opened this issue 6 years ago • 2 comments

Description

tradeModel calls quantmod:::modelReturn. Inside quantmod:::modelReturn, days.traded is calcualted by the following.

Browse[2]> n
debug: days.traded <- sum(abs(trade.signal), na.rm = TRUE)

This is incorrect. trade.signal is two column xts object.

Browse[2]> tail(trade.signal)
           Next.OpCl.IBM signal.zoo
2018-10-12  -0.001913187          1
2018-10-15   0.005342641          1
2018-10-16   0.015322200         -1
2018-10-17  -0.013540334          1
2018-10-18  -0.016424268          1
2018-10-19  -0.011863667          1

So, the current incorrect calculation of days.traded is the following.

Browse[2]> sum(abs(trade.signal), na.rm = TRUE)
[1] 393.4607

Expected behavior

A sufficient correct calculation of days.traded may be the following.

Browse[2]> sum(abs(trade.signal[, 2]), na.rm = TRUE)
[1] 391

Minimal, reproducible example

Full example (what I used above)

library(quantmod)
getSymbols("IBM", from = "2017-01-01", to = "2018-10-20")
# [1] "IBM"

m <- specifyModel(Next(OpCl(IBM)) ~ Lag(OpHi(IBM)))
m.built <- buildModel(m,method='rpart',training.per=c("2017-01-01","2017-04-01"))
# loading required package: rpart

# tradeModel calls quantmod:::modelReturn and I want to debug that
debug(quantmod:::modelReturn)

tm <- tradeModel(m.built,leverage=2)
tm

Session Info

Browse[2]> devtools::session_info()
- Session info ---------------------------------------------------------------
 setting  value
 version  R version 3.5.1 Patched (2018-10-09 r75424)
 os       Windows 10 x64
 system   x86_64, mingw32
 ui       RTerm
 language (EN)
 collate  English_United States.1252
 ctype    English_United States.1252
 tz       America/Chicago
 date     2018-10-25

- Packages -------------------------------------------------------------------
 package              * version date       lib source
 curl                   3.2     2018-03-28 [1] CRAN (R 3.5.1)
 devtools               2.0.0   2018-10-19 [1] CRAN (R 3.5.1)
 quantmod             * 0.4-13  2018-04-13 [1] CRAN (R 3.5.1)
 TTR                  * 0.23-4  2018-09-20 [1] CRAN (R 3.5.1)
 xts                  * 0.11-1  2018-09-12 [1] CRAN (R 3.5.1)
 zoo                  * 1.8-4   2018-09-19 [1] CRAN (R 3.5.1)

AndreMikulec avatar Oct 26 '18 00:10 AndreMikulec

Thanks for the report and reproducible example. I removed most of the packages from the devtools::sessoin_info() output, because most of the 30+ packages you had loaded/attached were not relevant to quantmod.

I'll take a quick look at this, but can't promise it will get much attention. The buildModel() and tradeModel() functionality in quantmod is incomplete. I don't personally use it, and do not have much time to invest in fixing it.

You correctly show that only the second column should be part of the sum(abs(...)) calculation. But how can you be sure the signals will always be the second column? What if the model has more columns of data? This will be more likely to get fixed if you can demonstrate what the correct behavior should be in a variety of use cases, preferably with unit tests.

joshuaulrich avatar Oct 26 '18 02:10 joshuaulrich

But how can you be sure the signals will always be the second column?

Because the first column is always the LHS of the formula ( and the LHS of the formula only every has one column. )

What if the model has more columns of data? 

The RHS (that has multiple columns) of the formula, is never ever in trade.signal.

AndreMikulec avatar Oct 27 '18 04:10 AndreMikulec