Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ATR is calculated wrongly and is extremely #135

Closed
baont opened this issue Dec 16, 2020 · 5 comments
Closed

ATR is calculated wrongly and is extremely #135

baont opened this issue Dec 16, 2020 · 5 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@baont
Copy link
Contributor

baont commented Dec 16, 2020

Base on this article https://www.investopedia.com/terms/a/atr.asp. The way atr is calculated is wrong because it only takes into account 'close' but leave out 'high', 'low'. Beside, when tested, this method runs extremely slow with little data.

@xmatthias
Copy link
Member

there's another issue where this is discussed in the strategy repo: freqtrade/freqtrade-strategies#30

Feel free to improve performance (but honestly, you'll have problems with that i'll think).
The way ATR is calculated requires a loop (current value is based on the result of the same calculation for the previous value).

Every indicator that requires a loop will automatically be a lot slower than any indicator that can be vectorized (calculated in one go across the whole timerange) - and there is very little possibility to optimize these calls.

That said, i just noticed that we have 2 different calculations of atr:
one in volatility.py (this will most likely be wrong as it seems to only use close - but uses the average_true_range directly from pyti - so having that fixed would require an issue against the pyti repository.

The second version is in indicators (which is vendored from qtpylib). At first glance, the calculation seems ... better aligned to investopedia ...

however please investigate in the linked issue above, as i think also this calculation does not match with tradingview (however i don't see tradingview as gold standard, maybe their implementation is wrong).

@xmatthias xmatthias added bug Something isn't working help wanted Extra attention is needed labels Dec 16, 2020
@baont
Copy link
Contributor Author

baont commented Dec 16, 2020

well i found this code on stackoverflow https://stackoverflow.com/questions/40256338/calculating-average-true-range-atr-on-ohlc-data-with-python and tested myself

def wwma(values, n):
    """
     J. Welles Wilder's EMA 
    """
    return values.ewm(alpha=1/n, adjust=False).mean()

def atr(df, n=14):
    data = df.copy()
    high = data[HIGH]
    low = data[LOW]
    close = data[CLOSE]
    data['tr0'] = abs(high - low)
    data['tr1'] = abs(high - close.shift())
    data['tr2'] = abs(low - close.shift())
    tr = data[['tr0', 'tr1', 'tr2']].max(axis=1)
    atr = wwma(tr, n)
    return atr

It's extremely extremely extremely fast

@xmatthias
Copy link
Member

it might be fast - but it's most likely not aligned to the calculation of investopedia - which does not mention the use of Wilders MA at all.

You can also use from technical.vendor.qtpylib.indicators import atr - which will also be extremely fast (as said, it's in technical twice for no real reason)

@baont
Copy link
Contributor Author

baont commented Dec 16, 2020

i could change to values.rolling(14).mean(). Haven't tested it yet

@xmatthias
Copy link
Member

as said, technical does contain an implementation of atr already, which is fast.
I'll remove the pyti mapper so it'll be clear which one to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants