joshuaulrich / TTR

Technical analysis and other functions to construct technical trading rules with R
GNU General Public License v2.0
330 stars 103 forks source link

passing parameter for ADX (smoothing) calculation #18

Closed openbmsjsc closed 1 year ago

openbmsjsc commented 8 years ago

In the code it seems that n for ADX calculation is the same as n for DX calculation.

maArgs <- list(n = n, ...)

Should we separate these 2? If we change here, we should probably change in addADX as well.

joshuaulrich commented 8 years ago

Can you please clarify? What do you want to separate, and why?

openbmsjsc commented 8 years ago

I think there are 2 parameters: DX period and Smoothing period (to calculate ADX, see https://www.linnsoft.com/techind/adx-avg-directional-movement, tab Preferences).

Therefore setting up 2 separate parameters will give more controls for technical analysts. In particular, I'm trying to replicate this (Charles Schaap - Using ADX to Trade Breakouts, Pullbacks, and Reversal Patterns) in R, which requires different DX period and ADX smoothing parameters.

Of course we should default the smoothing to DX period as with present.

Thanks.

openbmsjsc commented 8 years ago

I think there is another bug in ADX function (at ATR call).


Index: R/ADX.R
===================================================================
--- R/ADX.R (revision 186)
+++ R/ADX.R (working copy)
@@ -29,6 +29,7 @@
 #'High-Low-Close prices.
 #'@param n Number of periods to use for DX calculation (not ADX calculation).
 #'@param maType A function or a string naming the function to be called.
+#'@param nadx Number of periods to use for ADX (smoothing) calculation.
 #'@param \dots Other arguments to be passed to the \code{maType} function.
 #'@return A object of the same class as \code{HLC} or a matrix (if
 #'\code{try.xts} fails) containing the columns:
@@ -66,7 +67,7 @@
 #'
 #'@export
 "ADX" <-
-function(HLC, n=14, maType, ...) {
+function(HLC, n=14, maType, nadx=n, ...) {

   # Welles Wilder's Directional Movement Index

@@ -77,7 +78,7 @@
   DMIp <- ifelse( dH==dL | (dH< 0 & dL< 0), 0, ifelse( dH >dL, dH, 0 ) )
   DMIn <- ifelse( dH==dL | (dH< 0 & dL< 0), 0, ifelse( dH <dL, dL, 0 ) )

-  TR    <- ATR(HLC)[,"tr"]
+  TR    <- ATR(HLC, n= n)[,"tr"]
   TRsum <- wilderSum(TR, n=n)

   DIp <- 100 * wilderSum(DMIp, n=n) / TRsum
@@ -85,7 +86,7 @@

   DX  <- 100 * ( abs(DIp - DIn) / (DIp + DIn) )

-  maArgs <- list(n=n, ...)
+  maArgs <- list(n=nadx, ...)

   # Default Welles Wilder EMA
   if(missing(maType)) {
joshuaulrich commented 8 years ago

There's no bug in the ATR call. The n only matters for the average true range, but ADX only uses the raw true range from the ATR output.

Instead of your proposed patch, I think I would prefer a solution where the user could specify a separate n for the ADX via the maType argument. For example: maType = list("EMA", n = 20). See the RSI code for an example of how I've done this.

openbmsjsc commented 8 years ago

Another attempt. I'm still learning R so the code might not be optimal or R-styled. I also make it consistent if wilder is not specified when maType='EMA'. This seems to happen also in RSI, ATR, and maybe elsewhere. Perhaps we should look at them as well?

Tests below are for the current version of ADX

> identical(TTR::ADX(ttrc[,c("High", "Low", "Close")], n=20),TTR::ADX(ttrc[,c("High", "Low", "Close")], n=20, maType="EMA", wilder=T))
[1] TRUE
> identical(TTR::ADX(ttrc[,c("High", "Low", "Close")], n=20),TTR::ADX(ttrc[,c("High", "Low", "Close")], n=20, maType="EMA"))
[1] FALSE

> identical(RSI(ttrc[,'Close'], n = 10), RSI(ttrc[,'Close'], n = 10, maType = 'EMA', wilder=TRUE))
[1] TRUE
> identical(RSI(ttrc[,'Close'], n = 10), RSI(ttrc[,'Close'], n = 10, maType = 'EMA'))
[1] FALSE

Thank you for feedback.

index cb42038..90224eb 100644
--- a/R/ADX.R
+++ b/R/ADX.R
@@ -93,6 +93,19 @@ function(HLC, n=14, maType, ...) {
     maArgs$wilder <- TRUE
   }

+  if (is.list(maType)) {
+    # If MA function has 'n' arg, see if it's populated in maType;
+    # if it isn't, populate it with ADX's formal 'n'
+    if (!is.null( formals(maType[[1]])$n ) && is.null(maType$n)) {
+      maType$n <- n
+    }
+    maArgs <- maType[-1]
+    maType <- maType[[1]]
+  }
+  if (maType == 'EMA' && is.null(maArgs$wilder)) {
+    maArgs$wilder <- TRUE
+  }
+
   ADX <- do.call( maType, c( list(DX), maArgs ) )

   result <- cbind( DIp, DIn, DX, ADX )
openbmsjsc commented 8 years ago

I just wonder if you would consider to merge this patch?

ethanbsmith commented 1 year ago

I also make it consistent if wilder is not specified when maType='EMA'. This seems to happen also in RSI, ATR, and maybe elsewhere. Perhaps we should look at them as well?

the R language supports undefined parameter passing with ... which in this case are passed to the ma function, so you can just pass those parameters to the ATR call. see difference in the atr column output below:

> tail(ATR(HLC(getSymbols("SPY")), n = 20, maType = "EMA", wilder = T))
                tr      atr trueHigh  trueLow
2022-10-17 10.6991 8.890820 367.9799 357.2808
2022-10-18  8.6300 8.877779 375.4500 366.8200
2022-10-19  6.3000 8.748890 371.8500 365.5500
2022-10-20  8.0600 8.714445 372.6700 364.6100
2022-10-21 11.2600 8.841723 374.8000 363.5400
2022-10-24  6.9500 8.747137 380.0600 373.1100
> tail(ATR(HLC(getSymbols("SPY")), n = 20, maType = "EMA", wilder = F))
                tr      atr trueHigh  trueLow
2022-10-17 10.6991 9.701230 367.9799 357.2808
2022-10-18  8.6300 9.599208 375.4500 366.8200
2022-10-19  6.3000 9.284998 371.8500 365.5500
2022-10-20  8.0600 9.168331 372.6700 364.6100
2022-10-21 11.2600 9.367538 374.8000 363.5400
2022-10-24  6.9500 9.137296 380.0600 373.1100

i dont think there are any remaining issues here. just opportunities for documentation or possibly some refactoring