Repository navigation
Improve consolidator registration ergonomics: quote-to-trade bar adaptation, registration-time period validation, calendar periods in RegisterIndicator - #9675
Conversation
- Adapt trade bar consolidators and indicators to quote-only feeds (forex, cfd) by collapsing quote bars into mid-point trade bars with zero volume (new QuoteBarToTradeBarAdapter), instead of rejecting the registration with a type mismatch error - Validate fixed time span consolidator periods against the subscription period at registration time, mirroring the check PeriodCountConsolidatorBase performs when the first data point arrives, so invalid setups fail in Initialize instead of mid-backtest - Accept calendar periods (Calendar.WEEKLY/MONTHLY, custom functions) in RegisterIndicator and ResolveConsolidator, in C# overloads and in the Python register_indicator path, like Consolidate already does - Throw a clear ArgumentNullException for null symbols (e.g. Future.Mapped before the first mapping) instead of an NRE, and suggest Consolidate()/self.consolidate() in the type mismatch error
…or adapters the identity of the consolidator they wrap - QuoteDerivedTradeBar: trade bars collapsed from quote bars throw on Volume access instead of reporting zero - TradeBarConsolidator keeps consolidated bars volume-less when fed collapsed bars - ConsolidatorAdapterBase: adapters equal their wrapped consolidator and share its hash code - ConsolidatorIdentityComparer backs the registered consolidator collections so lookups work from either side - Period validation runs on the user's consolidator before it is wrapped
…y the registration paths - QuoteDerivedTradeBar reads the quote bar mid-point prices directly instead of going through Collapse() - Drop the per-registration trace log from SubscriptionManager.AddConsolidator - Python RegisterIndicator tries the calendar conversion before the throwing timedelta cast
4eccdf9 to
9dd410d
Compare
There was a problem hiding this comment.
Thanks, this is careful work. The null-symbol message, the Consolidate() hint and calendar periods in RegisterIndicator all look good. CI is green. I built the head commit and ran two small backtests on both master and this branch. Two behavior regressions came out of that, so I'm requesting changes.
1. The registration-time period check rejects consolidators that work today (blocking).
ValidateConsolidatorPeriod compares the period to subscription.Increment. The existing check in PeriodCountConsolidatorBase.Update compares it to the length of the bar that actually arrives (data.EndTime - data.Time), and those two differ for daily data. With the default DailyPreciseEndTime = true, a daily equity bar runs 09:30 to 16:00, so it is 6.5h long, not 1 day.
- Probe: SPY at daily resolution plus
new TradeBarConsolidator(TimeSpan.FromHours(12)), from 2013-10-07 to 2013-10-11. - master: runs and emits 4 consolidated bars.
- This branch: fails in
InitializewithUnable to add consolidator for symbol SPY: the consolidator period (12:00:00) is smaller than the daily subscription period (1.00:00:00).
Any fixed period between the session length and 1 day is affected, on any daily subscription with precise end times. The new check has to be no stricter than the runtime one. One option is to validate only intraday resolutions, where the increment equals the bar length. Another is to derive the daily bar length from exchange hours. Either way, please add a test case for this.
2. Quote-to-trade adaptation moves some failures from Initialize to the first bar (blocking).
This contradicts the PR's own goal of failing at registration.
- Volume indicators.
RegisterIndicator(eurusd, new OnBalanceVolume(), Resolution.Hour)used to throw a type mismatch inInitialize. Now it registers fine and throws on the first consolidated bar.VolumeIndicatorOnQuoteOnlyFeedFailsWhenReadingVolumeasserts exactly that. In live trading, that bar can arrive hours after deployment. - Code that reads volume only when it's there.
HeikinAshireadsVolumefrom anyTradeBar(Indicators/HeikinAshi.cs:111).BaseTimelessConsolidator's default volume selector does the same (BaseTimelessConsolidator.cs:75), as doIntradayVwapandSessionBar. All of them now throw on aQuoteDerivedTradeBar.- Probe: EURUSD at minute resolution plus
RegisterIndicator(eurusd, new HeikinAshi(), new TradeBarConsolidator(TimeSpan.FromHours(1))). - master: type mismatch in
Initialize. - This branch: runtime error
The trade bar for EURUSD was collapsed from a quote bar and has no volume ... Use ... an indicator that does not depend on volumeon the first bar. HeikinAshi doesn't depend on volume, so that message points users the wrong way. ClassicRenkoConsolidator<TradeBar>/RenkoConsolidator<TradeBar>on forex hit the same throw through the default volume selector.
- Probe: EURUSD at minute resolution plus
I'd keep every case that fails today failing in Initialize. Some possible ways:
- Adapt only when the consumer is known not to read volume.
- Give the collapsed bar zero volume plus a flag that volume-dependent indicators check at registration.
- Keep the type mismatch for registrations that pass an explicit
TradeBarConsolidator.
Whichever you pick, please cover the chained cases above with tests.
3. Scope. There's no linked issue, and the PR bundles four independent changes. Calendar periods, the null-symbol message and the mismatch hint are low risk and could merge on their own. The adaptation and the period validation change behavior and carry the issues above. I'd suggest splitting those two into their own PR.
Minor
ConsolidatorAdapterBase.Equalsisn't symmetric (adapter.Equals(wrapped)is true,wrapped.Equals(adapter)is false). The description calls this out. It's only safe as long as every collection holding adapters usesConsolidatorIdentityComparer.IndicatorBase.Consolidatorsholds the user's consolidator, so it's fine today. A test pinning that would help keep it that way.SubscriptionDataConfig.Consolidatorsnow exposes the adapter, not the consolidator the user registered, to anyone who iterates it.
Testing / evidence: The unit tests and regression algorithms cover the expected paths. They don't cover daily bars with precise end times, or consumers that read volume opportunistically. Once this is reworked, please also add the outcome of an end-to-end run for a forex algorithm that registers a trade bar consolidator and indicator.
Other repositories: The public API additions (the RegisterIndicator / ResolveConsolidator calendar overloads, ConsolidatorAdapterBase, QuoteBarToTradeBarAdapter, ConsolidatorIdentityComparer, the ConcurrentSet constructor, the BaseData.IsFillForward protected setter) reach the generated Python stubs on the next release. The documentation still needs an update for calendar periods in register_indicator and for how trade bar consolidators behave on forex/CFD.
Risk: risk:high. The PR adds to the public algorithm API and changes registration behavior users rely on, and the two regressions above are reproducible.
| private static void ValidateConsolidatorPeriod(Symbol symbol, IDataConsolidator consolidator, SubscriptionDataConfig subscription) | ||
| { | ||
| var period = (consolidator as ConsolidatorBase)?.FixedTimeSpanPeriod; | ||
| if (period.HasValue && period.Value < subscription.Increment) |
There was a problem hiding this comment.
subscription.Increment is 1 day for daily data, but with DailyPreciseEndTime a daily equity bar is only 09:30 to 16:00 long. The runtime check in PeriodCountConsolidatorBase.Update compares against data.EndTime - data.Time, so a 12h consolidator on daily SPY works on master (4 bars over 2013-10-07 to 2013-10-11) and is now rejected here in Initialize. This check needs to be no stricter than the runtime one.
| /// </summary> | ||
| public override decimal Volume | ||
| { | ||
| get => throw NoVolume(); |
There was a problem hiding this comment.
Some consumers read Volume from any TradeBar they receive: HeikinAshi (line 111), BaseTimelessConsolidator's default volume selector, IntradayVwap, and SessionBar. With this throw they now fail at runtime on adapted feeds. RegisterIndicator(eurusd, new HeikinAshi(), new TradeBarConsolidator(TimeSpan.FromHours(1))) throws in Initialize on master. On this branch it throws on the first bar, with a message telling the user to pick an indicator that doesn't depend on volume.
| // so we use a smarter selector as in other API methods | ||
| selectorToUse = consolidated => (T)(object)new IndicatorDataPoint(consolidated.Symbol, consolidated.EndTime, consolidated.Value); | ||
| } | ||
| else if (type == typeof(TradeBar) && consolidator.OutputType == typeof(QuoteBar) && selector == null) |
There was a problem hiding this comment.
For volume indicators (OBV, VWAP(symbol, period), ...) on forex/CFD, this branch turns an Initialize-time type mismatch into a runtime error on the first consolidated bar. In live trading that can be hours after deployment. Could volume-dependent indicators keep failing at registration?
Description
Consolidator registration on quote-only feeds (forex, cfd) and with mismatched periods fails late or needlessly. The engine can adapt type mismatches, validate periods at registration, accept calendar periods in
RegisterIndicator, and reject null symbols clearly.The change:
SubscriptionManager.AddConsolidatoradapts trade bar consolidators to quote-only feeds through a newQuoteBarToTradeBarAdapter.QuoteBaris collapsed into an internalQuoteDerivedTradeBar: mid-point prices, the fill forward flag kept, and reading or writingVolumethrowsInvalidOperationException.TradeBarConsolidatorkeeps its consolidated bar volume-less when fed collapsed bars.Volumethrows, throughRegisterIndicator,ConsolidateandSubscriptionManager.AddConsolidatoralike.HeikinAshi, the default volume selector of the Renko/Range consolidators,SessionBar) check the newTradeBar.HasVolumeand skip it on collapsed bars.QCAlgorithm.RegisterIndicator<T>does the same collapse at the indicator layer (e.g.self.mass(symbol, resolution=Resolution.HOUR)on a cfd), and registers the consolidator only after the output type check.Consolidatewith aTradeBarhandler andWarmUpIndicator(by period, resolution or automatic warm-up) are adapted the same way, so registration and warm-up agree.ConsolidatorAdapterBase, which equals the consolidator it wraps and shares its hash code, so users remove the consolidator they hold.ConsolidatorIdentityComparerbacksSubscriptionDataConfig.Consolidatorsand the scan dictionary, so lookups match from either side (ConcurrentSetnow accepts a comparer).Initialize.RegisterIndicator/ResolveConsolidatoraccept calendar rules (Calendar.WEEKLY,Calendar.MONTHLY, custom), in C# and Python; a Python type passed by mistake (e.g.timedeltawithout arguments) is rejected rather than taken as a calendar.Consolidate(), and a null symbol (e.g.Future.Mappedbefore the first mapping) throwsArgumentNullExceptionexplaining the likely cause.BaseData.IsFillForwardsetter is now protected so the collapsed bar can overrideClone(bool)without touching volume.Important
Known limits
adapter.Equals(wrapped)is true, butwrapped.Equals(adapter)is false.==/ReferenceEqualsbetween the adapter and the wrapped consolidator stay false.SubscriptionDataConfig.Consolidatorsand the scan dictionary).SubscriptionDataConfig.Consolidatorsyields the adapter for adapted registrations;IndicatorBase.Consolidatorsholds the consolidator the user registered.Related Issue
N/A
Motivation and Context
The engine has all the information to adapt or fail fast at registration, and the late period error can kill a backtest months of simulated time in.
Requires Documentation Change
register_indicatordocs can mention calendar periods. The quote-to-trade adaptation is transparent, except thatvolumeon the collapsed bars throws, which the registration warning states.How Has This Been Tested?
ConsolidatorAutoAdaptationRegressionAlgorithm(C# + Python): trade bar consolidator,MassIndexandHeikinAshion EURUSD receive collapsed bars, and the bars throw on volume.RegisterIndicatorWithCalendarRegressionAlgorithm(C# + Python): weekly RSI registered withCalendar.WEEKLYupdates when the week boundary is crossed.ConsolidatorPeriodValidationRegressionAlgorithm(C# + Python): 10-second consolidator on minute data is rejected inInitialize.QuoteDerivedTradeBarTests: mid-point collapse, volume get/set throw,HasVolumefalse, Renko tolerates the bars, fill forward flag kept, clones keep the type, consolidation keeps bars volume-less.SubscriptionManagerTests: adapter data flow and removal, Python adapted consolidator add/remove, adapter equality, comparer matching both ways, period validation cases (also through the adapter, and 7/12/23-hour periods accepted on daily data with precise end times), null-symbol message, mismatch suggestion.AlgorithmIndicatorsTests: calendar registration in C# and Python,MassIndexandHeikinAshion a quote-only feed update,OnBalanceVolumeon a quote-only feed throws on the first bar.AlgorithmIndicatorsTests: adapted registrations warn the user by resolution, trade consolidator, quote consolidator,Consolidateand directAddConsolidator; trade feeds do not warn.AlgorithmIndicatorsTests: the indicator keeps the consolidator the user registered while the subscription holds the adapter that equals it.AlgorithmIndicatorsTests: a type mismatch leaves no consolidator registered,timedeltaitself as third argument is rejected,Consolidatewith a trade bar handler on EURUSD receives collapsed bars.AlgorithmIndicatorsTests:MassIndexon EURUSD warms up by period, by resolution and through automatic warm-up.PythonIndicatorTests: updated messages.Types of changes
Checklist:
bug-<issue#>-<description>orfeature-<issue#>-<description>