braverock / quantstrat

289 stars 114 forks source link

Possible Error in "stoptrailing" order for OHLC data #130

Closed augur2 closed 3 years ago

augur2 commented 3 years ago

Hi,

it seems that a "stoptrailing" order ist not executed if a daily "low" price crosses the "Order.price", but is executed if the close price crosses the "Order.price".

For example:

A simple SMA-Strategy with Take Profit and Trailing Stop Loss. The strategy buys long on 2020-07-23. On 2020-08-05 a stoptrailing order is set at price: 373.33724148364. On 2020-08-07 low price is 367.9355 but the stoptrailing order is not executed. On 2020-08-14 a stoptrailing order is set at price: 411.61103087864. On 2020-08-19 close price is 406.4638 and the stoptrailing order is executed.

Data:

       date     open     high      low    close
 2020-07-22 245.0373 262.9850 242.4843 262.1907
 2020-07-23 262.3886 277.5835 261.0471 274.6890
 2020-07-24 274.7227 286.1928 269.2398 279.2154
 2020-07-25 279.0264 306.7410 279.0264 304.0568
 2020-07-26 303.6924 316.3863 300.2678 309.6436
 2020-07-27 309.6579 330.7012 309.6579 321.5141
 2020-07-28 321.8298 325.9060 307.7214 316.6573
 2020-07-29 316.5554 324.3808 313.1096 318.1909
 2020-07-30 318.1450 338.6312 315.7511 334.5866
 2020-07-31 334.6337 348.6114 329.3409 345.5546
 2020-08-01 345.7986 388.8480 343.5874 385.1997
 2020-08-02 385.5499 411.2283 357.1436 370.6717
 2020-08-03 371.1339 396.5070 369.3363 386.2952
 2020-08-04 386.1565 400.7006 382.9851 389.8755
 2020-08-05 389.7108 406.3040 386.2185 401.5906
 2020-08-06 401.5839 403.4887 392.6002 394.9620
 2020-08-07 395.2269 398.2490 367.9355 379.5129
 2020-08-08 379.5516 393.9874 377.3497 393.9874
 2020-08-09 395.3052 399.7371 385.8307 391.1205
 2020-08-10 391.0415 399.3759 391.0415 395.8876
 2020-08-11 395.8947 398.4789 370.8606 380.3841
 2020-08-12 380.0638 391.3123 367.9236 391.0242
 2020-08-13 390.8381 432.9046 379.7109 428.7418
 2020-08-14 428.6773 444.5778 423.3459 437.3978
 2020-08-15 437.5630 441.7546 429.8746 433.3549
 2020-08-16 433.3506 436.2658 415.0863 433.7866
 2020-08-17 433.9738 442.7350 422.6473 429.5313
 2020-08-18 429.6696 432.5803 419.6741 423.6693
 2020-08-19 423.7386 427.0246 396.6783 406.4638
 2020-08-20 406.7589 418.7344 404.0261 416.4398

Orderbook:

      date        Order.Qty     Order.Price   Order.Type Order.Side Order.Threshold Order.Status    Order.StatusTime Prefer Order.Set Txn.Fees             Rule       Time.In.Force
 2020-07-23 181.620304578563    262.38864728       market       long            <NA>       closed 2020-07-24 00:00:00   Open   ocolong        0        EnterLONG                    
 2020-07-24              all 532.96203490182        limit       long 258.23933649882     canceled 2020-08-19 00:00:00          ocolong        0   TakeProfitLONG                    
 2020-07-24              all 241.75597459464 stoptrailing       long -32.96672380836     replaced 2020-07-25 00:00:00    low   ocolong        0 StopTrailingLONG 2020-07-25 00:00:00
 2020-07-25              all 273.77427569264 stoptrailing       long -32.96672380836     replaced 2020-07-26 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-07-26              all 283.41960187664 stoptrailing       long -32.96672380836     replaced 2020-07-27 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-07-27              all 297.73449154464 stoptrailing       long -32.96672380836     replaced 2020-07-30 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-07-30              all 305.66446617664 stoptrailing       long -32.96672380836     replaced 2020-07-31 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-07-31              all 315.64464845964 stoptrailing       long -32.96672380836     replaced 2020-08-01 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-08-01              all 355.88125051964 stoptrailing       long -32.96672380836     replaced 2020-08-04 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-08-04              all 367.73390163464 stoptrailing       long -32.96672380836     replaced 2020-08-05 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-08-05              all 373.33724148364 stoptrailing       long -32.96672380836     replaced 2020-08-13 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-08-13              all 399.93786770164 stoptrailing       long -32.96672380836     replaced 2020-08-14 00:00:00          ocolong        0 StopTrailingLONG                    
 2020-08-14              all 411.61103087864 stoptrailing       long -32.96672380836       closed 2020-08-19 00:00:00          ocolong        0 StopTrailingLONG                    

Code snippet:

add.rule(strategy.st, name = "ruleSignal",
         arguments = list(sigcol = "long",
                          sigval = TRUE,
                          ordertype = "market",
                          ordecalc_slde = "long",
                          replace = FALSE,
                          # numeric order quantity for entry
                          orderqty = 10,
                          # set order sizing function to use position limits
                          osFUN = osInvestAll,
                          fac = 3,
                          TxnFees = 0,#"txnFUN",
                          prefer = "Open",
                          orderset = "ocolong"),
         type = "enter",
         label = "EnterLONG")

add.rule(strategy.st,
         name = "ruleSignal",
         arguments = list(sigcol = "short",
                          sigval = TRUE,
                          ordertype = "market",
                          ordecalc_slde = "long",
                          replace = FALSE,
                          orderqty = "all",
                          TxnFees = 0,#"txnFUN",
                          prefer = "Open"),
         type = "exit",
         label = "ExitLong")
add.rule(strategy.st, name = 'ruleSignal',
         arguments=list(sigcol='long' , sigval=TRUE,
                        replace=FALSE,
                        ordecalc_slde='long',
                        ordertype='limit', tmult=TRUE, threshold=quote(.takeprofit),
                        TxnFees=0,
                        orderqty='all',
                        orderset='ocolong'
         ),
         type='chain', parent='EnterLONG',
         label='TakeProfitLONG',
         enabled=FALSE
)

add.rule(strategy.st,
         name = "ruleSignal",
         arguments = list(sigcol = "long",
                          sigval = TRUE,
                          orderside = "long",
                          ordertype = "stoptrailing",
                          orderqty = "all",
                          prefer = "low", 
                          threshold = quote(trailingStopPercent), 
                          time.in.force= 24*60*60*1,
                          tmult = T, 
                          TxnFees = 0,
                          orderset = "ocolong",
                          replace = F),
         type = "chain",
         parent = "EnterLONG",
         enabled = T,
         label = "StopTrailingLONG")

See also :

"[...] The confusing part for that was using the OHLC data and mixing the close price as a "filter" rather than just highest high lowest low. **Also for the fills as well, as a close below the stop limit order is not really realistic. As you can be filled by the low of the day and still have the close above (high of the day and close below for a short), in reality you would have obtained a fill in the market. This is assuming that the stoptrailing order is in the market as it executes at the limit price. But in the back test you would still be maintaining a position as the order would not be filled. This leads to some disparity in the results as only winners would not be filled and continue higher but any losers would be cut off properly. When in reality both would have been closed. I believe this backtesting behavior biases the returns upwards when using the stoptrailing order.[...] "

(https://mailman.stat.ethz.ch/pipermail/r-sig-finance/2016q1/013789.html)

jaymon0703 commented 3 years ago

Hi @augur2 there is no minimal reproducible example so i cannot comment much. However i do notice that the stop trailing order has a TIF of 1 day, so possibly that order set on 2020-08-05 was not live on 2020-08-07?

augur2 commented 3 years ago

Hi @jaymon0703, thank you for your response.

I don't think TIF is the problem, otherwise on 2020-08-19 the stoptrailing order wouldn't also be live.

I made a minimal reproducible example:

example.txt

In addition, if you change the close on 2020-08-07 from 379.5129 to 360 the stoptrailing order is executed.

Thank you!

jaymon0703 commented 3 years ago

Thanks. Well, we dont assess the prices on 2020-08-07...we jump straight from 2020-08-05 to 2020-08-13. I think this was partially identified in https://github.com/braverock/quantstrat/issues/116. Thanks for the report...i will take a closer look a bit later.

jaymon0703 commented 3 years ago

I suspect this is a bug, and we need to update the dindexOrderProc() function. On 2020-08-05 which is the 15th index in the market data, the dindexOrderProc() function determines the next relevant index to review is 23 or 2020-08-13 which is when the security makes a new high. I believe the below code is where we need to make a change. We need to check for any Lows/Highs which are lower/greater than our order price AND before the index returned using the existing .firstCross() function for determining the next high.

if (orderType %in% "stoptrailing") {
    orderThreshold <- as.numeric(Order[1L, "Order.Threshold"])
    if (orderQty > 0) {
      relationship <- "lt"
      newOrderPrice <- orderPrice - abs(orderThreshold)
    }
    else {
      relationship <- "gt"
      newOrderPrice <- orderPrice + abs(orderThreshold)
    }
    out$move_order <- .firstCross(mktPrice, newOrderPrice, 
      relationship, start = curIndex + 1L)
  }

I can take a stab during my spare time either tomorrow or on the weekend.

jaymon0703 commented 3 years ago

Ok i think i have found the issue. Its not in the above code block, but just above that in the dindexOrderProc() function.

In the below snippet, we are looking to see if the trailing stop order price was breached. The relationship is correctly lte. However, the mktPrice is the Close price...which is passed as such into the dindexOrderProc() function. So we will need to see if we can pass in the correct series for that stoptrailing mktPrice to compare with the orderPrice.

out$cross <- .firstCross(mktPrice, orderPrice, relationship, 
    start = curIndex + 1L)

When checking to see when the next highest high or lowest low occurs in order to index that bar so we can potentially reset any remaining open orders on that timestamp, the block in my previous comment comes into play. The relationship for the .firstCross() function is reversed and the abs(orderThreshold) added back to the orderPrice for the comparison. In this case the mktPrice is Close and it should be High for long open orders and Low for short open orders, the opposite of what it should be for checking whether our stoptrailing order price was crossed.

The reason the stoptrailing order did not result in a trade on 2020-08-07 is because the comparison was the Close price, and not the Low. I will test a change hopefully this weekend.

jaymon0703 commented 3 years ago

Hi @augur2 made a small change to potentially resolve the issue. Commit is https://github.com/braverock/quantstrat/commit/010830d5da12e25b96122aa28594f60179191db2. Please test with the new branch? Your script now generates a trade on 2020-08-07 at 370.521976.

> out <- applyStrategy(strategy = strategy.st, portfolios = portfolio.st, initEq=initEq)
[1] "2020-07-24 00:00:00 df_example_xts 0.0109201023431992 @ 274.7227"
[1] "2020-08-07 00:00:00 df_example_xts -0.0109201023431992 @ 370.521976"

This change will require some more testing before i will be comfortable the issue is resolved. I will add more tests in due course. For now using install_github("braverock/quantstrat", ref = "130_stoptrailing_bug_fix") will give you the package with the latest changes for the potential bug. Thanks!

augur2 commented 3 years ago

Thank you @jaymon0703 it's working now!

jaymon0703 commented 3 years ago

Thanks for confirming. The mktPrice in the dindexOrderProc() function will need to change for determining the index the current stoptrailing order needs to be moved to. Currently we will compare the orderPrice + abs(orderThreshold) with the Low, but we should be comparing with the High. I am double checking this as it will impact testing with BBO data as well.

augur2 commented 3 years ago

Hi @jaymon0703, i don't know if this is another bug. In the example above the orderbook looks like this now:

        date          Order.Qty Order.Price   Order.Type Order.Side Order.Threshold Order.Status    Order.StatusTime Prefer Order.Set Txn.Fees             Rule       Time.In.Force
1 2020-07-23 0.0109201023431992    262.3886       market       long            <NA>       closed 2020-07-24 00:00:00   Open   ocolong        0        EnterLONG                    
2 2020-07-24                all  532.962038        limit       long      258.239338     canceled 2020-08-07 00:00:00          ocolong        0   TakeProfitLONG                    
3 2020-07-24                all  241.755976 stoptrailing       long      -32.966724     replaced 2020-07-25 00:00:00    low   ocolong        0 StopTrailingLONG 2020-07-25 00:00:00
4 2020-07-25                all  273.774276 stoptrailing       long      -32.966724     replaced 2020-07-27 00:00:00          ocolong        0 StopTrailingLONG                    
5 2020-07-27                all  297.734476 stoptrailing       long      -32.966724     replaced 2020-08-01 00:00:00          ocolong        0 StopTrailingLONG                    
6 2020-08-01                all  355.881276 stoptrailing       long      -32.966724     replaced 2020-08-06 00:00:00          ocolong        0 StopTrailingLONG                    
7 2020-08-06                all  370.521976 stoptrailing       long      -32.966724       closed 2020-08-07 00:00:00          ocolong        0 StopTrailingLONG                    

The stoptrailing order on 2020-08-01 is replaced on 2020-08-06 but not on 2020-08-02.

Shouldn't it be replaced on 2020-08-02 as there is a new high of 411.2283?

Thank you!

jaymon0703 commented 3 years ago

Yes...we will need to reference the High for determining when to move stoptrailing orders (for long entry positions, vice versa for short entry positions). Right now we are using Low (previously Close) for both determining whether the order traded and whether or not the stoptrailing order price needs to be adjusted. That was my previous comment. I will update the code and docs this weekend, and let you know so you can test?

augur2 commented 3 years ago

Perfect, thanks.

jaymon0703 commented 3 years ago

Hi @augur2 please reinstall this feature branch with my latest commit and test?

Transaction output from your script below.

> out <- applyStrategy(strategy = strategy.st, portfolios = portfolio.st, initEq=initEq)
[1] "2020-07-24 00:00:00 df_example_xts 0.0109201023431992 @ 274.7227"
[1] "2020-08-03 00:00:00 df_example_xts -0.0109201023431992 @ 371.1339"

Thanks!

augur2 commented 3 years ago

Hi @jaymon0703 , it's working now. :)

Is there a possibility to choose the column which is used to determine if stoptrailing order price needs to be adjusted?

For example using the low and not the high (actually like it was before your last change).

Thanks

jaymon0703 commented 3 years ago

I guess we could do that. In some ways, the prefer argument in ruleSignal should be doing this already. I will need to take a closer look. This will become a feature request (if @braverock agrees), and we can use your script for our automated tests.

braverock commented 3 years ago

I'm not sure what exactly you're asking for. the prefer argument is passed to getPrice, and we've used it to decide when to match a trade. I worry that if you pass e.g. Low to a trailing long stop, you'll miss the stop (because it wasn't moved high enough) and then people will complain. It seems like the new code covers 98%+ of all reasonable cases, even if you're going to make the false argument that matching on OHLC data makes sense at all (markets trade BBO, not OHLC).

jaymon0703 commented 3 years ago

I have to agree with @braverock here. The extra work required to make configurable which column is ultimately used is unlikely to add sufficient value. Moving the price up for higher highs where the stoptrailing order is an exit related to a long position, or moving the price down for a lower low where the stoptrailing order is an exit related to a short position makes the most sense to me. Will focus on some tests and documentation of the latest change next.

jaymon0703 commented 3 years ago

This feature branch is merged. Closing the issue.