anandanand84 / technicalindicators

A javascript technical indicators written in typescript with pattern recognition right in the browser
MIT License
2.18k stars 563 forks source link

MACD calculation is wrong: should be (slow period - fast period) and not vice versa #44

Closed dexbg closed 7 years ago

dexbg commented 7 years ago

The MACD value is reversed - i.e. the formula is (slow period - fast period) and not vice versa

anandanand84 commented 7 years ago

Just verified it in wikipedia and stockcharts.com both use fast - slow.

https://en.wikipedia.org/wiki/MACD

http://stockcharts.com/school/doku.php?id=chart_school:technical_indicators:moving_average_convergence_divergence_macd

anandanand84 commented 7 years ago

@dexbg If you find this correct you can close the issue, else provide me a resource and we can discuss and if it is correct I am happy to change.

dexbg commented 7 years ago

Sorry, my mistake.

heekah7 commented 6 years ago

anandanand84, I cannot thank you enough for the amazing library. Unfortunately, dexbg is right, when I compare the generated MACD is different from Yahoo financial. Maybe they're wrong but it worths investigating.

anandanand84 commented 6 years ago

@heekah7 can you send me a screenshot of the difference, did you look at cryptocurrency or other? Can you check if yahoo finance is different from https://cryptotrading-hub.com if it is cryptocurrency, cryptotrading hub uses this library

anandanand84 commented 6 years ago

I just verified against trading view it seems to be correct, can you verify again if the parameters are the same with both yahoo and the library

screen shot 2017-12-08 at 12 36 37 pm screen shot 2017-12-08 at 12 36 27 pm

bitcoinvsalts commented 6 years ago

I am not able to get the same MACD between binance and this MACD value. Is this formula right?

anandanand84 commented 6 years ago

It uses the formula from http://stockcharts.com/school/doku.php?id=chart_school:technical_indicators:moving_average_convergence_divergence_macd and it has test cases from stock charts if it has one. If you see an issue in the code, I am happy to change it.

anandanand84 commented 6 years ago

Can you send your calculation, I see it is the same with binance and this library? image

anandanand84 commented 6 years ago

@jsappme @jaggedsoft https://cryptotrading-hub.com uses the technical indicators from this library and it seems to match with binance. Can you compare the two charts you may be comparing the wrong parameters.

jaggedsoft commented 6 years ago

@anandanand84 I couldn't say for sure, seems to work great. I love this library, thanks for sharing.

anandanand84 commented 6 years ago

@jsappme I did go through the code again since there were lot of people reporting this, I did find out an issue. By default it uses simple moving average that is where people get confused.

try below

macd({
    values            : [] //close prices
    fastPeriod        : 5,
    slowPeriod        : 8,
    signalPeriod      : 3,
    SimpleMAOscillator: false, //this defaults to true that is where the problem is
    SimpleMASignal    : false, //this defaults to true that is where the problem is
})

Feel free to reopen the issue if you find a mismatch after using simple ma as false.

bitcoinvsalts commented 6 years ago

it s working but I had to play with my histo prices a bit like this:


var macdInput = {
                values            : _.reverse([close, ...minute_prices[pair]]).map(i=>parseFloat(i)),
                fastPeriod        : 12,
                slowPeriod        : 26,
                signalPeriod      : 9 ,
                SimpleMAOscillator: false,
                SimpleMASignal    : false
            }

            var current_macd = MACD.calculate(macdInput)[MACD.calculate(macdInput).length-1].histogram
anandanand84 commented 6 years ago

You can also use reversedInput, so you nolonger have to reverse back and forth.

var macdInput = {
   reversedInput : true
}