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
970 stars 246 forks source link

is issue #949 really fixed? #1263

Open mihakralj opened 1 day ago

mihakralj commented 1 day ago

What happened?

Seems like initial value for TR (True Range) is still zero. When the loop starts, your code sets the prevClose to the current close and then skips the TR calculation entirely (due to the continue statement)

This means that in your implementation, the TR for the first bar is effectively left as 0 and even the initial ATR average is calculated from the second bar onwards. I thought this was fixed in #949? Going through commit code, changes that were implemented in #949 are not in the main branch now.

the xlst test for True Range is also skipping the first bar. This is contrary to official Wilder's definition of TR and ATR: https://user-images.githubusercontent.com/31756078/163031211-e3b8b4c7-b453-464a-b561-44171ca2b89e.png

Code usage

No response

Log output

No response

DaveSkender commented 1 day ago

v2.5.0 published on 11/15/2023 and this was merged on 11/26 … it’s never been deployed! I remember discussing this a while back.

DaveSkender commented 1 day ago

I’ve redeployed a new version 2.5.1 to be safe, but now see that these merged on 11/26/2022. These changes have been available since 2.4.1.

I think we resolved some of Wilder’s inconsistencies here:

mihakralj commented 1 day ago

Here is what I see on main branch in Atr.Series.cs:

            if (i > 0)
            {
                hmpc = Math.Abs(q.High - prevClose);
                lmpc = Math.Abs(q.Low - prevClose);
            }
            else
            {
                prevClose = q.Close;
                continue;
            }

The else clause is incomplete - it doesn't define tr (as q.High-q.Low) when i=0 - and continues into the second loop (i=1)

I did think we resolved it, but clearly the first Tr calculated in your code is 0 instead of high-low...

DaveSkender commented 1 day ago

Yes, that is the actual fix. The old uncorrected version used to set TR as q.High-q.Low when i=0. Reading down through Wilder’s docs, he uses this “guess” value to initialize inconsistently, so it’s difficult to say which is better by purely relying on his notes. I think he actually made a mistake, the guess value isn’t mathematically consistent, so we just skip the first period.

And it should be null for i=0 with our output convention, not 0