Closed bdowling closed 1 year ago
Yes good spot
"Account.Connection.Now" this infers its related to PlayBack versus realtime. So this could be replaced by ATSQuadroStrategyBase.Now
This looks pretty inefficient admittedly at line 4115 //probably platform TimeZone set Core.Globals.Now;
private DateTime now = Core.Globals.Now;
[Browsable(false)]
public DateTime Now
{
get
{
now = (Cbi.Connection.PlaybackConnection != null ? Cbi.Connection.PlaybackConnection.Now : Core.Globals.Now);
if (now.Millisecond > 0)
now = Core.Globals.MinDate.AddSeconds((long)Math.Floor(now.Subtract(Core.Globals.MinDate).TotalSeconds));
return now;
}
}
In current work as far as i can recall i'm using this pattern more so where replay sensitive DateTime is required - cant see why we would need to use the Core.Globals.Now - except for platform timezone date i guess.
DateTime DateNow
{ get=> (Cbi.Connection.PlaybackConnection != null ? Cbi.Connection.PlaybackConnection.Now : DateTime.Now);
}
//private DateTime now = Core.Globals.Now;
[Browsable(false)]
public DateTime Now
{
get
{
return (Cbi.Connection.PlaybackConnection != null ? Cbi.Connection.PlaybackConnection.Now :DateTime.Now);
}
}
Curious why you marked this as invalid.
I've been testing with these changes and it mostly looks good. What I did find that the TradeWorkflowTimeout
was actually working by design. In Playback, what was happening before was the timeout was in realtime, while the data was obviously flowing faster than realtime.
When I fixed it, I noticed it because my Entry orders (which were Limit, not market) were closing before they executed... Having timeouts on the Entry orders is probably a good thing, but it should probably be configurable separately from the 10 seconds default of TradeWorkFlowTimeout
(max is 30 seconds) to...
the label was meant to mean the code doesn't seem right eg invalid code not your discussion was invalid :-) will mark it as new label "code focus"
The fix i would do is to remove redundant dates and only use the NT date where it is necessary such as playback and stick with real dates elsewhere.
"In Playback, what was happening before was the timeout was in realtime, while the data was obviously flowing faster than realtime." yup so the playback date there etc.
Generally Playback is a different workflow event type and should be treated more as a backtest and not do the full realtime workflow - but there are some areas of overlap - this is by design.
"When I fixed it," Not quite on the same page what did you fix?
TradeTimeouts are configurable yes. Depending on the mode of operation are ok or need widening - on low powered machines and faster scalping a longer timeout is reequired. https://github.com/MicroTrendsLtd/NinjaTrader8/issues?q=timeout
Types of TimeOut
Workflow TimeOut For Realtime TimeOuts based on processing getting stuck are implemented as a failsafe and should not be used in replay as it wll skip time so orders will be cancelled immediately etc the same would apply to backtest -
NOTE: if a workflow timeout is kicking in for replay or backtest this would be a bug not a feature.
TTL - Order Time to Live - Duration by TimeSeries The concept of cancel an order after a duration by bar timeseries can be used for both replay backtest and realtime -this uses the bars to cancel the order not a datetime comparison
"Fix Playback mode to follow trusted realtime codepaths for consistency"
Playback and Realtime wll never share the same workflow by design
Playback and backtest do not need a fault tolerant realtime workflow
and do not provide the same events to work in the workflow
Caveats such as Workflow TimeOut
Speed it would be slow
it would also break as the events dont exist to push the state forwards
There might be some overlap or areas which can use common methods paths Will look at the proposed code changes etc
worfklow timeouts This is related: https://github.com/MicroTrendsLtd/NinjaTrader8/issues/36
I noticed that logging was using real-time instead of the historical playback data time.
On inspection, I do see 3 different methods for getting sense of "
Now
"My gut feeling is some of these should perhaps be changed, but a quick and dirty replace to have all of the ones that could directly use the local base
Now()
did result in errors, such asOnMarketData >> TWF >> ErrorTimeOut
seeming to fire rather quickly.Not groking all this code just yet, so I figured I'd post this issue for discussion/consideration.