joshuaulrich / TTR

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

TTR::ADX and "Error in TTR::EMA not enough non-NA values" #102

Closed yogat3ch closed 3 years ago

yogat3ch commented 3 years ago

Hi @joshuaulrich, Hope this finds you well - I've happened across an error while running ADX as described in the title.

The reprex is simple:

d <- setNames(purrr::map_dfc(1:3, ~rnorm(14)), c("high","low","close"))
TTR::ADX(d, n = 7)

This can be remedied by passing on the n value supplied to ADX to ATR instead of taking the default of 14. I feel like this would be expected behavior and the odd values I've been getting for the ADX for quite some time now might be explained by the fact that no matter the n value passed to ADX, the ATR values it uses are based on a window of 14 periods.

Here's the line 79.

Here's a pull request for the change.

I submitted it and seconds later realized I submitted it to master instead of a branch, but hopefully it's minor enough where that shouldn't matter.

Edit 2020-09-05T16:06:35: It looks like this introduces an error of the same nature at line 23 where the second call of EMA does not have sufficient data. I'm not sure how best to remedy this for non-standard values of n smaller than the default and achieve accurate values for the ADX as it seems like the C code for ema might need to be adapted. Your expertise will be appreciated as to how best to go about this.

Update 2020-09-05T16:22:01: It looks like changing the maType to TTR::SMA after making the modification to line 79 such that n is passed to ATR gives a result without error. This would suggest that the underlying implementation for ema might indeed need to be adjusted.

Thanks for your time!

My sessionInfo jic:

R version 3.5.3 (2019-03-11)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18362)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252 
[2] LC_CTYPE=English_United States.1252   
[3] LC_MONETARY=English_United States.1252
[4] LC_NUMERIC=C                          
[5] LC_TIME=English_United States.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods  
[7] base     

other attached packages:
[1] magrittr_1.5         qf_0.0.0.9999       
[3] RevoUtils_11.0.3     RevoUtilsMath_11.0.0

loaded via a namespace (and not attached):
  [1] websocket_1.1.0      colorspace_1.4-1    
  [3] slider_0.1.5.9000    ellipsis_0.3.1      
  [5] class_7.3-15         timetk_2.1.0        
  [7] rprojroot_1.3-2      fs_1.4.1.9000       
  [9] logspline_2.1.16     rstudioapi_0.11     
 [11] listenv_0.8.0        furrr_0.1.0.9002    
 [13] remotes_2.2.0        dials_0.0.8         
 [15] bit64_0.9-7          prodlim_2019.11.13  
 [17] fansi_0.4.1          lubridate_1.7.9     
 [19] codetools_0.2-16     splines_3.5.3       
 [21] pkgload_1.1.0        jsonlite_1.7.0      
 [23] workflows_0.1.2      pROC_1.16.2         
 [25] packrat_0.5.0        anytime_0.3.8       
 [27] yardstick_0.0.6      tune_0.1.1.9000     
 [29] compiler_3.5.3       httr_1.4.2          
 [31] tictoc_1.0           backports_1.1.6     
 [33] assertthat_0.2.1     Matrix_1.2-15       
 [35] cli_2.0.2            prettyunits_1.1.1   
 [37] tools_3.5.3          gtable_0.3.0        
 [39] glue_1.4.2           dplyr_1.0.0         
 [41] Rcpp_1.0.5.2         DiceDesign_1.8-1    
 [43] vctrs_0.3.2          iterators_1.0.12    
 [45] parsnip_0.1.2        timeDate_3043.102   
 [47] gower_0.2.2          stringr_1.4.0       
 [49] globals_0.12.5       ps_1.3.4            
 [51] testthat_2.99.0.9000 lifecycle_0.2.0.9000
 [53] devtools_2.3.1       future_1.18.0       
 [55] tsibble_0.9.0        MASS_7.3-51.1       
 [57] zoo_1.8-8            scales_1.1.1.9000   
 [59] ipred_0.9-9          parallel_3.5.3      
 [61] yaml_2.2.1           quantmod_0.4-16     
 [63] curl_4.3             memoise_1.1.0       
 [65] ggplot2_3.3.2        rpart_4.1-13        
 [67] stringi_1.5.2        RSQLite_2.2.0.9000  
 [69] desc_1.2.0           foreach_1.5.0       
 [71] TTR_0.24.2           lhs_1.0.2           
 [73] warp_0.1.0           pkgbuild_1.1.0      
 [75] lava_1.6.7           rlang_0.4.7.9000    
 [77] pkgconfig_2.0.3      lattice_0.20-38     
 [79] purrr_0.3.4.9000     recipes_0.1.10      
 [81] bit_1.1-15.2         tidyselect_1.1.0    
 [83] processx_3.4.3       plyr_1.8.6          
 [85] R6_2.4.1             generics_0.0.2      
 [87] DBI_1.1.0            pillar_1.4.6.9000   
 [89] withr_2.2.0          xts_0.12-0          
 [91] RPushbullet_0.3.3    survival_2.43-3     
 [93] nnet_7.3-12          tibble_3.0.3.9000   
 [95] HDA_0.0.0.9000       crayon_1.3.4        
 [97] catchr_0.2.2.9000    utf8_1.1.4          
 [99] doFuture_0.9.0       AlpacaforR_1.0.0    
[101] usethis_1.6.1        grid_3.5.3          
[103] data.table_1.13.0    blob_1.2.1          
[105] callr_3.4.3          digest_0.6.25.1     
[107] tidyr_1.1.0          GPfit_1.0-8         
[109] munsell_0.5.0        sessioninfo_1.1.1 
ethanbsmith commented 3 years ago

only the tr value is used from the call to ATR. the n parameter is only used to calculate the average on the tr and should not impact the value of raw tr output column.

I think this has come up before. might we worth just adding a comment on this line in the code

yogat3ch commented 3 years ago

@ethanbsmith Ah, I see, indeed the n does not impact the tr values. Any ideas why the C code for ema errors when there's just enough values for n?

TTR::ATR(data.frame(h = rnorm(8), l = rnorm(8), c = rnorm(8)), n = 7)

Error in EMA(c(NA, 0.685597607387481, 0.409541520808519, 0.223128827893709, : not enough non-NA values

joshuaulrich commented 3 years ago

Any ideas why the C code for ema errors when there's just enough values for n?

Probably an off-by-one error in the NA check. I can investigate tomorrow.

joshuaulrich commented 3 years ago

Your question about ATR is a duplicate of #75, so I closed this with the fix to EMA (and ZLEMA).