MicroTrendsLtd / NinjaTrader8

NinjaTrader8 Components Strategies and Trading tools
MIT License
72 stars 17 forks source link

Fix Playback mode to follow trusted realtime codepaths for consistency #92

Closed bdowling closed 1 year ago

bdowling commented 1 year ago

This change may certainly need some review, as I do not have reference/understanding as to why this alternative code path was chosen for Playback mode.

 if (IsHistoricalTradeOrPlayBack || IsStrategyUnSafeMode)

I found a lot of instances where Playback was following the "IsStrategyUnsafeMode"; While not clear on the purpose, With all the warnings about not using UnsafeMode unless you fully understood the code, I was surprised this was being used in Playback by default. I would like to be able to backtest with confidence that close to the same code paths are used in live.

I think I caught the cases where a Historical (but not Playback) conditional is needed, and adjusted accordingly.

btw, One side effect that actually caused me to dig deeper into this was that the OrdersStopLoss list was never being populated, which prevent the more elegant looping use case in the Strategy... it doesn't populate in the Unsafe codepath.

foreach (Order o in OrdersStopLoss.Where(o => IsOrderIsActive(o)).ToList<Order>()) {
}

I have been testing with this update for a couple of days, and have not run into any issues in playback or live modes.

Oh, and this addresses the "Entry Order Timeout" I discovered in #88, now my entry orders don't timeout. (Still looking for the OrderTTL references you mentioned).

Note that this PR also includes the Now changes in #91 so that you can see it as a fully working branch.

Closes #88

MicroTrendsTom commented 1 year ago

"Fix Playback mode to follow trusted real-time codepaths for consistency" Note: Playback and Realtime will never share the exact same workflow by design Playback and back test do not need a fault tolerant real-time 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 don’t 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.

So when you say fix "code paths for consistency" - not sure what that relates to? Do you mean you want playback to be treated the same way as real-time? or is there some resourceful method or snippet which can be used -which was not etc.?

There is overlap but only a subset so it cant use the same paths by design/ Playback should not follow all the same paths as real-time - nor should back test. Playback skips events and skips time etc...

Playback uses a subset of the workflow 1)The NT CEP events it uses are not the same as real-time 2) It would slow down playback for no good reason - playback is lean and mean 3)Some events would not fire and push it forwards etc. 4)Errors and caveats - such as workflow monitoring - time comparisons would cause order cancellations and the timeout process to fire 5) playback does not require error handling and fault tolerance to the same degree as real-time

Trade Engine Time Outs for example:
Realtime uses this - back test and playback should not need to use the fault tolerance and recovery modes of real-time.

This is related: https://github.com/MicroTrendsLtd/NinjaTrader8/issues/36

Workflow Timeout For Realtime Timeouts based on processing getting stuck are implemented as a failsafe and should not be used in replay as it will skip time so orders will be cancelled immediately etc. the same would apply to back test -

NOTE: if a workflow timeout is kicking in for replay or back test this would be a bug not a feature - or surplus to requirement and should be bypassed conditionally etc

TTL - Order Time to Live - Duration by Time Series The concept of cancel an order after a duration by bar timeseries can be used for both replay back test and real-time -this uses the bars to cancel the order not a datetime comparison

Playback mode vs real time Playback is not treated the same way as real-time it is more like back test - so we should not be using timeouts based on datetime on orders or using the part of the workflow that deals with this for playback - use Bars to cancel an order. So we have to make sure that is still the case or indeed raise a bug and fix for playback etc.

Unsafe Mode So the idea with this was to allow fast execution with no checks in real-time This is ok for Playback as playback does not use the same event model as real-time In a real-time situation its possible to get fills and cancels and erroneous positions - the Trade Workflow will catch this and deal with it.. in Unsafe mode it will skip checks and assume the users is in attendance and will rectify any overfills or underfills.. etc. Note: if this was not used in playback its because its surplus as far as I recall etc.

foreach (Order o in OrdersStopLoss.Where(o => IsOrderIsActive(o)).ToList<Order>()) {
}

What maybe valid is a mode of playback that simulates realtime for the purpose of going through all the steps to test. for testing of realtime workflow i usually fire a long or short for example from the Gui in the sample hybrid strategy then step through the debugger in VStudio 2019/2022 etc.

Anyway all good, will have a look at the code but very busy on a different project etc

MicroTrendsTom commented 1 year ago

"Still looking for the OrderTTL references you mentioned" i also cant see it in there - so in fact this exists in a layer above in a derived project whereby for example a limit entry is cancelled after n Bars - my bad... So all good Workflow Timeouts... was the one here