search

DaveSkender / Stock.Indicators

Stock Indicators for .NET is a C# NuGet package that transforms raw equity, commodity, forex, or cryptocurrency financial market price quotes into technical indicators and trading insights. You'll need this essential data in the investment tools that you're building for algorithmic trading, technical analysis, machine learning, or visual charting.
https://dotnet.StockIndicators.dev
Apache License 2.0
988 stars 246 forks source link

GetDonchian off by one time period #1195

Closed pgrimstrup closed 5 months ago

pgrimstrup commented 6 months ago

the problem

Using the GetDonchian method to calculate Donchian Channels appears to be off by one time period. The new high/low is shown on the candle that is after the candle that triggered the new high/low.

Interestingly, the sample image on the Donchian Indicator page also shows this behavior. https://dotnet.stockindicators.dev/indicators/Donchian/#content

Checking the code, there appears to be an off-by-one error with the inner-loop calculation. The condition is currently

                    for (int j = i - lookbackPeriods; j < i; j++)

The condition should be (as it should include the current time period in the high/low calculation)

                    for (int j = i - lookbackPeriods + 1; j <= i; j++)

Correcting this appears to fix the problem, and new high/lows now begin on the correct candle.

Please determine if other charting platforms are accurate before comparing to this library. We're unable to debug their problems for you.

Yes - comparing against Donchian Channel usage on TradingView where new high/lows start on the candle, not after it.

my situation

workaround

Implemented my own GetDonchian for now. Looking forward to an update.

DaveSkender commented 6 months ago

@pgrimstrup thank you for contributing and sharing this issue. I'll look into this further soon to assess the situation. Here are a few similar issues.

I also see this in my list of known TradingView problems that I've previously reviewed.

DaveSkender commented 6 months ago

@pgrimstrup any change in your perspective after reviewing my previous analyses here? I’d concluded that the TradingView author didn’t implement it in accordance with the original authors intent; and provided a few other reputable sources to back that up.

DaveSkender commented 5 months ago

@pgrimstrup, do you have any open issues here? I don't believe there's an bug or issue with this library.

Please reopen if you disagree, or otherwise let us know how we can help.