Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop loss on the same day the position was opened #119

Open
DAtek opened this issue Jul 29, 2020 · 11 comments
Open

Stop loss on the same day the position was opened #119

DAtek opened this issue Jul 29, 2020 · 11 comments
Labels
bug Something isn't working

Comments

@DAtek
Copy link

DAtek commented Jul 29, 2020

Expected Behavior

Strategy closes the position at 720

Actual Behavior

Strategy closes the position on next candle at open price, 705.58

Steps to Reproduce

Run the following code. You can see the wrong behaviour on 2012-10-18

from backtesting import Backtest, Strategy
from backtesting.test import GOOG


class DummyStrategy(Strategy):
    def init(self):
        pass

    def next(self):
        if f"{self.data.index[-1]}" == "2012-10-17 00:00:00":
            self.buy(sl=720)


if __name__ == "__main__":
    data = GOOG.tail(100)
    backtest = Backtest(data, DummyStrategy, cash=10000)
    result = backtest.run()
    backtest.plot()

Additional info

  • Backtesting version: 0.2.0
@DAtek DAtek changed the title Stop loss not applied on same day the position was opened Stop loss didn't close the position on the same day the position was opened Jul 29, 2020
@kernc
Copy link
Owner

kernc commented Jul 29, 2020

Strategy.next() is assumed to be called between bars. Strategy.buy() just places the buying order, but orders are executed and turned into trades, as mentioned in the quick start manual, the soonest on the next available bar.

Documentation improvements welcome.

@kernc kernc closed this as completed Jul 29, 2020
@kernc kernc reopened this Jul 29, 2020
@kernc
Copy link
Owner

kernc commented Jul 29, 2020

Nevermind, I misunderstood.

I guess the reason SL isn't hit on the same bar is that we iterate over a copy of orders state:

for order in list(self.orders): # type: Order

so once we turn an order into a trade:

# Open a new trade
if need_size:
self._open_trade(adjusted_price, need_size, order.sl, order.tp, time_index)

the subsequent SL order that was placed in the queue isn't picked up in the same broker iteration.

@kernc kernc added the bug Something isn't working label Jul 29, 2020
@kernc
Copy link
Owner

kernc commented Jul 29, 2020

But imagine an order on that day placed with self.buy(stop=758, sl=720), i.e. the order is only entered once the stop price is hit (which is above open price ($755)). This then makes it ambiguous whether the stop price was hit before the SL price (in which case we enter and shortly after exit a losing trade) or after (in which case we just enter the trade and leave it open, along with its SL order, waiting for another bar). We can draw assumptions from the chart in this particular case, but not the general case — we don't have intra-candle data.

Screenshot_2020-07-29_17-35-51

Due to this ambiguity, and the resolving complexity, I'm kinda leaning to wontfix.

Have a better idea?

@kernc kernc changed the title Stop loss didn't close the position on the same day the position was opened Stop loss on the same day the position was opened Jul 29, 2020
@DAtek
Copy link
Author

DAtek commented Jul 29, 2020

I don't think the strategy should allow stop orders for exactly that reason. I would limit the entry point of a position to open and close, and the users would have still enough freedom to implement their logic to decide whether the strategy should open a position in the next iteration or not. In the previous versions it worked perfectly.

@kernc
Copy link
Owner

kernc commented Aug 2, 2020

In the previous versions it worked perfectly.

Glad you think so. 😊

The following commit should restore the previous behavior for plain market orders. Thanks.

@kernc kernc closed this as completed in bcf8eda Aug 2, 2020
@DAtek
Copy link
Author

DAtek commented Aug 3, 2020

Thank you for the quick fix!!! 😊

@rokups
Copy link

rokups commented Sep 24, 2022

Actually i think we can reasonably improve same candle entries and exits as we can reliably deduce some intra-candle movements.
image
Consider this image. We always know that wicks guarantee movement up and down. We also know candle body has at least one direction of price movement, in direction of the candle.

Given all that, we can always do this:

  • If a candle touches stop-loss price - always fill that in, ignoring candle movements.
  • If trade entry and take-profit prices are within wicks - close trade in-profit.
  • If trade entry and take-profit are within same candle - close trade in-profit only when candle matches trade direction (green candle for longs, red candle for shorts).
  • If neither of these conditions are satisfied, then current behavior should be maintained and take-profit order should be filled on a next eligible candle. This includes:
    • Long trade where limit entry is below open price of red candle and take-profit is in upper wick, because price may go up from the open, reach TP and only then fill an order and proceed down.
    • Short trade where limit entry is above open of green candle and take-profit is in lower wick, because price may go down from the open, reach TP and only then fill an order and proceed up.

I am new to the codebase so im still not sure how to correctly wrangle this in. Still, thought ill post it to get some feedback, while i am familiarizing myself with the code.

@zeshansari
Copy link

guys i am having trouble with following code, i set buying order with a condition so it should take when close price hit the given level and it does but take buy position at the wrong level which is not given anywhere but one thing is there that it open order before my tp and close position at earliest with profit of 0.5 points only. Although as per my understanding it should open position at close price when the giving condition is true i.e. 3903 but it is open position at 3911 and closing it 3911.5. can anyone help how can i resolve this problem?

def trade_level(self):
if self.data.close <= 3903 +1 and self.data.close >= 3903 - 1:
self.buy(sl= 3903 - 2, tp= 3903 + 9)

Goblincomet pushed a commit to Goblincomet/forex-trading-backtest that referenced this issue Jul 5, 2023
@PilotGFX
Copy link

PilotGFX commented Jul 15, 2023

if it is not possible to sell in to strength with a real limit order that executes intra candle, but always have to wait for a close or open, will simply make all the results quite unrealiable as its not close to any real world situation. Can anyone guide me on how to implement this functionality properly into the library?

@kernc
Copy link
Owner

kernc commented Aug 27, 2023

This is the relevant code part:

# Open a new trade
if need_size:
self._open_trade(adjusted_price,
need_size,
order.sl,
order.tp,
time_index,
order.tag)
# We need to reprocess the SL/TP orders newly added to the queue.
# This allows e.g. SL hitting in the same bar the order was open.
# See https://github.com/kernc/backtesting.py/issues/119
if order.sl or order.tp:
if is_market_order:
reprocess_orders = True
elif (low <= (order.sl or -np.inf) <= high or
low <= (order.tp or -np.inf) <= high):
warnings.warn(
f"({data.index[-1]}) A contingent SL/TP order would execute in the "
"same bar its parent stop/limit order was turned into a trade. "
"Since we can't assert the precise intra-candle "
"price movement, the affected SL/TP order will instead be executed on "
"the next (matching) price/bar, making the result (of this trade) "
"somewhat dubious. "
"See https://github.com/kernc/backtesting.py/issues/119",
UserWarning)
# Order processed
self.orders.remove(order)
if reprocess_orders:
self._process_orders()

I wonder why we exclude limit orders here ... 🤔🤨
if is_market_order:
reprocess_orders = True

Maybe that should be an if True: ...?

@kernc kernc reopened this Aug 27, 2023
@PabloCanovas
Copy link

PabloCanovas commented Mar 24, 2024

There are several cases here. I agree on that when price reachs both (TP and SL) within the same candle the result is unreliable, although I would go the route @rokups suggested, making SL trigger before than TP for green candles and TP before SL for red candles (if the trade is long).

But IMO at the very least it should be allowed to trigger the TP/SL when only one child order is triggered within the opening candle, take profit or stop loss.

So for instance:

  • trade_on_close = True
  • set a buy limit at $10 with SL = 9 and TP = 11
  • previous close was $10.5
  • next candle open =9.5, low = 9.2, high = 12 and close 10.7

Actual behaviour
Currently the library is opening the trade correctly at $10, but is waiting to close the trade at 10.7 (the close price) even though TP price was reached within the candle and without SL getting triggered whatsoever.

Desired behaviour:
Trade should close at TP price before the close.

The very same argument works the other way around obviously, when SL gets triggered on the same candle of the limit buy but TP price is above the high of the candle (for long trades) so there is no confusion about what child order gets triggered first.
I'd also argue the same should be true for (long) stop buys although I haven't tested that case thouroughly.

So this is how code would look like IMO (new code between *******):

# Open a new trade
if need_size:
    self._open_trade(adjusted_price,
                     need_size,
                     order.sl,
                     order.tp,
                     time_index,
                     order.tag)

# We need to reprocess the SL/TP orders newly added to the queue.
# This allows e.g. SL hitting in the same bar the order was open.
# See https://github.com/kernc/backtesting.py/issues/119

if order.sl or order.tp:
*********************************
    if (low <= (order.sl or -np.inf) <= high or low <= (order.tp or -np.inf) <= high): 
        reprocess_orders = True
*********************************

self.orders.remove(order)

if reprocess_orders:
    self._process_orders()```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants