Extend LeastSquaresMovingAverage to accept a benchmark - #9371
Conversation
AlexCatarino
left a comment
AlexCatarino
left a comment
There was a problem hiding this comment.
Thank you, @0xpinara.
Please do not add multiple commits from other pull requests. Rebase with master to include only relevant changes.
I've left a few comments to improve the implementation.
| /// <summary> | ||
| /// The reference symbol to regress against, if provided. | ||
| /// </summary> | ||
| private readonly Symbol _referenceSymbol; |
There was a problem hiding this comment.
If the default _referenceSymbol is Symbol.None, you can remove the null checks. See line 122.
| /// <summary> | ||
| /// Rolling window of reference symbol data points. | ||
| /// </summary> | ||
| private readonly RollingWindow<IndicatorDataPoint> _referenceWindow; |
There was a problem hiding this comment.
If the default _referenceWindow is new(0), _referenceWindow.IsReady is always true (see IsReady)
| .ToArray(); | ||
|
|
||
| double[] xValues; | ||
| if (_referenceSymbol != null && _referenceWindow.IsReady) |
There was a problem hiding this comment.
If default _referenceWindow is new(0), we can use _referenceWindow.Size != 0 && _referenceWindow.IsReady; and also use only one if-block.
decimal x = Period;
double intercept, slope;
if (_referenceWindow.Size != 0 && _referenceWindow.IsReady)
{
var xValues = _referenceWindow
.OrderBy(i => i.EndTime)
.Select(i => Convert.ToDouble(i.Value))
.ToArray();
x = _referenceWindow[0].Value;
(intercept, slope) = Fit.Line(x: xValues , y: series);
}
else
{
(intercept, slope) = Fit.Line(x: _t, y: series);
}
Intercept.Update(input.EndTime, intercept.SafeDecimalCast());
Slope.Update(input.EndTime, slope.SafeDecimalCast());
// Calculate the fitted value corresponding to the input
return Intercept.Current.Value + Slope.Current.Value * x;
Add new constructors that accept a reference Symbol, allowing LSMA to regress the target against a benchmark instead of time. Add corresponding LSMA overload in QCAlgorithm.Indicators.cs following the Alpha indicator pattern for dual-symbol registration. Use Symbol.None and RollingWindow(0) as defaults to simplify null checks. Use SafeDecimalCast and tuple deconstruction for Fit.Line results. Resolves QuantConnect#6984
0009d5e to
b4b272b
Compare
|
Hi @AlexCatarino, thanks for the feedback. I've pushed the fixes:
Let me know if anything else needs changing. |
Martin-Molinero
left a comment
Martin-Molinero
left a comment
There was a problem hiding this comment.
Hey! Sharing some concerns and requests 👍
| { | ||
| if (input.Symbol == _referenceSymbol) | ||
| { | ||
| _referenceWindow.Add(input); |
There was a problem hiding this comment.
Don't think this will work correctly, if main symbol comes first and later reference ( should rely on data coming in order) it wont be used for the same time, this requires some time awareness, can see similar multi symbol indicators and how they work.
Also generally I think ideally we should compare the indicator values in a test against some external source? like we usually do, at least trying to find one
Minor also but shouldn't create an unrequired rolling window _referenceWindow = new(0);
|
Closing for now until review address, feel free to reopen 👍 |
|
Hey @Martin-Molinero, thanks for the feedback! I'll work on fixing these when I get some time and reopen the PR. |
|
Sorry for the long delay on this. I couldn't reopen the PR so I opened #9761 |
3 participants
Description
LeastSquaresMovingAveragethat accept aSymbol referenceSymbolparameterLSMA(symbol, reference, period, resolution, selector)overload inQCAlgorithm.Indicators.csAlphaindicator pattern for dual-symbol data routing and registrationRelated Issue
Resolves #6984
Motivation and Context
Previously, LSMA only regressed price against time. Users who wanted to regress against a benchmark (e.g. SPY) had no built-in way to do so. This was specifically requested by a maintainer in the issue comments.
Requires Documentation Change
Yes — the new LSMA overload should be documented.
How Has This Been Tested?
WithReferenceRegressesAgainstBenchmark— feeds a perfect y=2x+1 relationship, verifies slope=2, intercept=1, projected value=11WithReferenceIsNotReadyUntilBothWindowsFull— verifies IsReady requires both target and reference windowsWithReferenceResetsProperly— verifies full state resetWithoutReferenceBehavesIdentically— confirms no regression in original behaviorTypes of changes
Checklist
feature-6984-lsma-benchmark-reference