From c802a9ea80e24fcbd757fad8418132dfae1a27be Mon Sep 17 00:00:00 2001 From: Miha Kralj Date: Tue, 9 Dec 2025 16:00:04 -0500 Subject: [PATCH] fix: address code review issues in indicators and core components --- lib/core/ringbuffer/RingBuffer.cs | 16 ++++++++++++++++ lib/core/tbarseries/tbarseries.cs | 10 ++++------ lib/core/tvalue/tvalue.cs | 4 ++-- lib/feeds/csv/CsvFeed.cs | 10 +++++++--- lib/trends/alma/Alma.cs | 6 +++++- lib/trends/dema/Dema.cs | 4 ++++ lib/trends/ema/Ema.cs | 4 ++++ lib/trends/kama/Kama.cs | 7 +------ lib/trends/lsma/Lsma.cs | 4 ++-- lib/trends/sma/Sma.cs | 11 +++++++---- lib/trends/trima/Trima.cs | 2 +- lib/trends/wma/Wma.cs | 2 +- 12 files changed, 54 insertions(+), 26 deletions(-) diff --git a/lib/core/ringbuffer/RingBuffer.cs b/lib/core/ringbuffer/RingBuffer.cs index 9e04801e..136a26a6 100644 --- a/lib/core/ringbuffer/RingBuffer.cs +++ b/lib/core/ringbuffer/RingBuffer.cs @@ -82,6 +82,22 @@ public sealed class RingBuffer : IEnumerable get => _sum; } + /// + /// Recalculates the sum by iterating over all elements. + /// Useful for correcting floating-point drift after many updates. + /// + public double RecalculateSum() + { + double sum = 0; + var span = GetSpan(); + for (int i = 0; i < span.Length; i++) + { + sum += span[i]; + } + _sum = sum; + return sum; + } + /// /// Average of all elements in the buffer. /// Returns 0 if buffer is empty. diff --git a/lib/core/tbarseries/tbarseries.cs b/lib/core/tbarseries/tbarseries.cs index 64e6443a..b9fd649e 100644 --- a/lib/core/tbarseries/tbarseries.cs +++ b/lib/core/tbarseries/tbarseries.cs @@ -129,12 +129,10 @@ public class TBarSeries : IReadOnlyList throw new ArgumentException("All arrays must have the same length"); } - _t.AddRange(tArr); - _o.AddRange(oArr); - _h.AddRange(hArr); - _l.AddRange(lArr); - _c.AddRange(cArr); - _v.AddRange(vArr); + for (int i = 0; i < tArr.Length; i++) + { + Add(tArr[i], oArr[i], hArr[i], lArr[i], cArr[i], vArr[i]); + } } public IEnumerator GetEnumerator() diff --git a/lib/core/tvalue/tvalue.cs b/lib/core/tvalue/tvalue.cs index 01bfca5a..c38fe930 100644 --- a/lib/core/tvalue/tvalue.cs +++ b/lib/core/tvalue/tvalue.cs @@ -24,7 +24,7 @@ public readonly struct TValue : IEquatable [MethodImpl(MethodImplOptions.AggressiveInlining)] public TValue(DateTime time, double value) { - Time = time.Ticks; + Time = time.Kind == DateTimeKind.Utc ? time.Ticks : time.ToUniversalTime().Ticks; Value = value; } @@ -38,7 +38,7 @@ public readonly struct TValue : IEquatable public override string ToString() => $"[{AsDateTime:yyyy-MM-dd HH:mm:ss}, {Value:F2}]"; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool Equals(TValue other) => Time == other.Time && Math.Abs(Value - other.Value) < 1e-9; + public bool Equals(TValue other) => Time == other.Time && Value == other.Value; public override bool Equals(object? obj) => obj is TValue other && Equals(other); public override int GetHashCode() => HashCode.Combine(Time, Value); diff --git a/lib/feeds/csv/CsvFeed.cs b/lib/feeds/csv/CsvFeed.cs index 4602fce8..3a3350b3 100644 --- a/lib/feeds/csv/CsvFeed.cs +++ b/lib/feeds/csv/CsvFeed.cs @@ -70,8 +70,9 @@ public class CsvFeed : IFeed var line = dataLines[i]; var parts = line.Split(','); + int originalLineNumber = dataLines.Count - i + 1; if (parts.Length != 6) - throw new FormatException($"Invalid CSV format at line {i + 2}. Expected 6 columns, found {parts.Length}"); + throw new FormatException($"Invalid CSV format at line {originalLineNumber}. Expected 6 columns, found {parts.Length}"); try { @@ -89,7 +90,7 @@ public class CsvFeed : IFeed } catch (Exception ex) when (ex is FormatException or OverflowException) { - throw new FormatException($"Failed to parse CSV line {i + 2}: {line}", ex); + throw new FormatException($"Failed to parse CSV line {originalLineNumber}: {line}", ex); } } @@ -153,7 +154,7 @@ public class CsvFeed : IFeed var result = new TBarSeries(count); // Find starting index - int startIndex = 0; + int startIndex = -1; for (int i = 0; i < _data.Count; i++) { if (_data[i].Time >= startTime) @@ -163,6 +164,9 @@ public class CsvFeed : IFeed } } + if (startIndex == -1) + return result; + // Collect bars matching interval long expectedTime = startTime; int collected = 0; diff --git a/lib/trends/alma/Alma.cs b/lib/trends/alma/Alma.cs index 923068fc..4b81866d 100644 --- a/lib/trends/alma/Alma.cs +++ b/lib/trends/alma/Alma.cs @@ -26,6 +26,8 @@ namespace QuanTAlib; public sealed class Alma : ITValuePublisher { private readonly int _period; + private readonly double _offset; + private readonly double _sigma; private readonly double[] _weights; private readonly double _weightSum; private readonly RingBuffer _buffer; @@ -62,6 +64,8 @@ public sealed class Alma : ITValuePublisher throw new ArgumentException("Sigma must be greater than 0", nameof(sigma)); _period = period; + _offset = offset; + _sigma = sigma; _buffer = new RingBuffer(period); _weights = new double[period]; Name = $"Alma({period}, {offset:F2}, {sigma:F2})"; @@ -129,7 +133,7 @@ public sealed class Alma : ITValuePublisher var tSpan = CollectionsMarshal.AsSpan(t); var vSpan = CollectionsMarshal.AsSpan(v); - Calculate(source.Values, vSpan, _period); + Calculate(source.Values, vSpan, _period, _offset, _sigma); source.Times.CopyTo(tSpan); // Restore state diff --git a/lib/trends/dema/Dema.cs b/lib/trends/dema/Dema.cs index 0bb9b9c5..33a76427 100644 --- a/lib/trends/dema/Dema.cs +++ b/lib/trends/dema/Dema.cs @@ -57,6 +57,7 @@ public sealed class Dema : ITValuePublisher private EmaState _p_state2 = EmaState.New(); private double _lastValidValue; + private double _p_lastValidValue; public string Name { get; } public TValue Last { get; private set; } @@ -93,11 +94,13 @@ public sealed class Dema : ITValuePublisher { _p_state1 = _state1; _p_state2 = _state2; + _p_lastValidValue = _lastValidValue; } else { _state1 = _p_state1; _state2 = _p_state2; + _lastValidValue = _p_lastValidValue; } // EMA1 @@ -302,6 +305,7 @@ public sealed class Dema : ITValuePublisher _p_state1 = EmaState.New(); _p_state2 = EmaState.New(); _lastValidValue = 0; + _p_lastValidValue = 0; Last = default; } } diff --git a/lib/trends/ema/Ema.cs b/lib/trends/ema/Ema.cs index 1f35351d..d8d06a45 100644 --- a/lib/trends/ema/Ema.cs +++ b/lib/trends/ema/Ema.cs @@ -56,6 +56,7 @@ public sealed class Ema : ITValuePublisher private State _state = State.New(); private State _p_state = State.New(); private double _lastValidValue; + private double _p_lastValidValue; /// /// Display name for the indicator. @@ -134,10 +135,12 @@ public sealed class Ema : ITValuePublisher if (isNew) { _p_state = _state; + _p_lastValidValue = _lastValidValue; } else { _state = _p_state; + _lastValidValue = _p_lastValidValue; } double val = GetValidValue(input.Value); @@ -305,6 +308,7 @@ public sealed class Ema : ITValuePublisher _state = State.New(); _p_state = _state; _lastValidValue = 0; + _p_lastValidValue = 0; Last = default; } } diff --git a/lib/trends/kama/Kama.cs b/lib/trends/kama/Kama.cs index fd3ed0bc..4ec59f4a 100644 --- a/lib/trends/kama/Kama.cs +++ b/lib/trends/kama/Kama.cs @@ -177,12 +177,7 @@ public sealed class Kama : ITValuePublisher // Use static Calculate for performance var outputSpan = new double[len]; - Calculate(source.Values, outputSpan, _period, - (int)(2.0 / _fastAlpha - 1), (int)(2.0 / _slowAlpha - 1)); // Reverse calc periods from alphas? - // Actually better to pass alphas or periods. - // The static method signature should match constructor params. - - // Wait, I need to pass periods to static method. + // fastPeriod = 2/fastAlpha - 1. int fastPeriod = (int)Math.Round(2.0 / _fastAlpha - 1); int slowPeriod = (int)Math.Round(2.0 / _slowAlpha - 1); diff --git a/lib/trends/lsma/Lsma.cs b/lib/trends/lsma/Lsma.cs index b2b8c2e3..6b034529 100644 --- a/lib/trends/lsma/Lsma.cs +++ b/lib/trends/lsma/Lsma.cs @@ -252,6 +252,8 @@ public sealed class Lsma : ITValuePublisher int windowSize = Math.Min(len, _period); int startIndex = len - windowSize; + Reset(); + // Initialize lastValidValue if (startIndex > 0) { @@ -269,8 +271,6 @@ public sealed class Lsma : ITValuePublisher _lastValidValue = 0; } - Reset(); - for (int i = startIndex; i < len; i++) { double val = GetValidValue(source.Values[i]); diff --git a/lib/trends/sma/Sma.cs b/lib/trends/sma/Sma.cs index c06f42ba..34cc4f9e 100644 --- a/lib/trends/sma/Sma.cs +++ b/lib/trends/sma/Sma.cs @@ -103,7 +103,7 @@ public sealed class Sma : ITValuePublisher if (_buffer.IsFull && _tickCount >= ResyncInterval) { _tickCount = 0; - _sum = _buffer.Sum(); + _sum = _buffer.RecalculateSum(); } } @@ -386,9 +386,12 @@ public sealed class Sma : ITValuePublisher public void Reset() { _buffer.Clear(); - var resetSum = 0; - _sum = resetSum; - Last = default; + _sum = 0; + _p_sum = 0; + _p_lastInput = 0; + _lastValidValue = 0; + _p_lastValidValue = 0; _tickCount = 0; + Last = default; } } diff --git a/lib/trends/trima/Trima.cs b/lib/trends/trima/Trima.cs index 81c455be..97ff8b22 100644 --- a/lib/trends/trima/Trima.cs +++ b/lib/trends/trima/Trima.cs @@ -156,7 +156,7 @@ public sealed class Trima : ITValuePublisher source.Times.CopyTo(tSpan); // Restore state - int lookback = _p1 + _p2; + int lookback = _p1 + _p2 - 1; int startIndex = Math.Max(0, len - lookback); Reset(); diff --git a/lib/trends/wma/Wma.cs b/lib/trends/wma/Wma.cs index d1720be8..a75d87b8 100644 --- a/lib/trends/wma/Wma.cs +++ b/lib/trends/wma/Wma.cs @@ -188,7 +188,7 @@ public sealed class Wma : ITValuePublisher _p_sum = _sum; _p_wsum = _wsum; - _p_lastInput = source.Values[len - 1]; + _p_lastInput = _lastValidValue; _p_lastValidValue = _lastValidValue; Last = new TValue(tSpan[len - 1], vSpan[len - 1]);