search

trufanov-nok / ta-lib-rt

TA-Lib RT is a fork of TA-Lib that provides additional API for incremental calculation of indicators without reprocessing whole data.
http://ta-lib.org
83 stars 31 forks source link

MACD discrepancy between talib and talib-rt #12

Closed mdukaczewski closed 4 years ago

mdukaczewski commented 4 years ago 
    closes = np.asarray( [2,3,4,5], dtype=np.float64)

    macd, signal, hist = talib.MACD(closes, fastperiod=3, slowperiod=2, signalperiod=1)
    print("talib", signal)

    _, state = talibrt.MACD_StateInit(fastperiod=3, slowperiod=2, signalperiod=1)
    _, _, signal, _ = talibrt.MACD_BatchState(state, closes)

signalperiod=1 is allowed with talib, but talib-rt gives: Exception: TA_MACD_StateInit function failed with error code 2: Bad Parameter (TA_BAD_PARAM) I'm not sure if this is intentional?

trufanov-nok commented 4 years ago

Hmm... good catch. After a brief look I'm not sure if it's a bug or some fancy trick. MACD does accepts signalperiod=1. But it calls EMA, and EMA doesn't accept timeperiod = 1. The original talib code ignores the result of EMA's lookback which is -1. And calls the variant of EMA implementation that don't permorm input params checks... Also they have +1 to lookback return... So it's either:

  1. MACD shouldn't accept signalperiod<2. And that's a talib's bug.
  2. MACD should work with signalperiod=1 and then the code relies on the fact that lookback returns -1 to signal about error. Which is weird.

I need to check up with MACD and EMA specifications, and perhaps alternative implementations to make a decision.

Btw, it looks like this is a known issue: https://sourceforge.net/p/ta-lib/bugs/84/ Mario is the author of the ta-lib. He suggested options to fix 12 years ago, but didn't resolve it in the code...

trufanov-nok commented 4 years ago

Ok, my current thoughts are following. The problem affects 6 indicators: MACD, MACDEXT, MACDFIX, STOCH, STOCHF and STOCHRSI. I believe that their results for optIn**Period == 1 in original talib are incorrect (didn't check that yet) and won't match to talib-rt results after I made the proposed fix. That's because they're return wrong Lookback value. So, I consider the ignorance of EMA lookback function call result as a bug. Especially if it sums -1 with something. I shall fix that even if the timeperiod checks in EMA will be relaxed and this affects nothing.

Another thing that I intended to do is to change the check for optInTimePeriod < 2 to optInTimePeriod < 1 for following indicators: SMA, EMA, WMA, DEMA, TEMA, TRIMA, KAMA, T3. These indicators could be called with optInTimePeriod = optInSignalPeriod = 1 from TA_MACDEXT() with help of TA_MA() and from STOCH* indicators. All of them are listed as a possible values of TA_MAType here. There is one more: MAMA, but TA_MAMA() has no optInTimePeriod argument. Instead it's called by TA_MA() with hardcoded params (0.5, 0.05).

Another question is: Am I allowed to do this change? I've checked with yahoo finance web charts. They allow to add a Moving Average indicator and choose one of several types for it as well as set a timeperiod equal to 1. The enumeration of MA types didn't exactly match to what we have for TA_MAType in TA-Lib. Some are there, some are missing and TA-Lib have some that didn't implemented by Yahoo. I've added all possible MAs to Yahoo chart. Most of them, except one, are drown on chart. The one that seems to not possible to calculate for timeperiod = 1 is TSMA (MA with type Time Series) - I'm not sure what is it and we don't have it in TA-Lib. Among the rest of MA with timeperiod = 1 output of almost all are equal to incoming data. So they just average nothing. With two exceptions: VMA (variable MA) and VIDYA. And we don't have such MAs in TA-Lib.

So I made an experiment and regenerate the code with optInSignalPeriod < 1 instead of optInSignalPeriod < 2 for named indicators. Then I checked they via wrapper. It looks like all of them behave as expected - do nothing and return output values that equal to input for timeperiod = 1. Except for KAMA (Kaufman Adaptive Moving Average). Unfortunately, Yahoo have no KAMA implementation, so it's results for timeperiod = 1 should be doublechecked somehow.

Amritshr commented 1 year ago

for me too these function are not working in release mode TA_STOCHRSI TA_MACD TA_ADXR TA_MACDFIX TA_MAVP TA_PPO TA_STOCH TA_STOCHF TA_DEMA TA_TEMA TA_APO TA_TRIX i use VC++ 2022

trufanov-nok commented 1 year ago

@Amritshr are you using TA-Lib RT, built from sources from github's master branch?

Amritshr commented 1 year ago

Oops, I think I post in the wrong forum. no, I am not using RT.