Merge pull request #11248 from xmatthias/fix/backtest_max_detail_2

Improved Backtest timeframe-detail execution logic

docs/backtesting.md

@@ -508,7 +508,12 @@ To utilize this, you can append `--timeframe-detail 5m` to your regular backtest
 freqtrade backtesting --strategy AwesomeStrategy --timeframe 1h --timeframe-detail 5m
 ```
 
-This will load 1h data as well as 5m data for the timeframe. The strategy will be analyzed with the 1h timeframe, and Entry orders will only be placed at the main timeframe, however Order fills and exit signals will be evaluated at the 5m candle, simulating intra-candle movements.
+This will load 1h data (the main timeframe) as well as 5m data (detail timeframe) for the selected timerange.
+The strategy will be analyzed with the 1h timeframe.
+Candles where activity may take place (there's an active signal, the pair is in a trade) are  evaluated at the 5m timeframe.
+This will allow for a more accurate simulation of intra-candle movements - and can lead to different results, especially on higher timeframes.
+
+Entries will generally still happen at the main candle's open, however freed trade slots may be freed earlier (if the exit signal is triggered on the 5m candle), which can then be used for a new trade of a different pair.
 
 All callback functions (`custom_exit()`, `custom_stoploss()`, ... ) will be running for each 5m candle once the trade is opened (so 12 times in the above example of 1h timeframe, and 5m detailed timeframe).
 
@@ -520,6 +525,27 @@ Also, data must be available / downloaded already.
 !!! Tip
     You can use this function as the last part of strategy development, to ensure your strategy is not exploiting one of the [backtesting assumptions](#assumptions-made-by-backtesting). Strategies that perform similarly well with this mode have a good chance to perform well in dry/live modes too (although only forward-testing (dry-mode) can really confirm a strategy).
 
+??? Sample "Extreme Difference Example"
+    Using `--timeframe-detail` on an extreme example (all below pairs have the 10:00 candle with an entry signal) may lead to the following backtesting Trade sequence with 1 max_open_trades:
+
+    | Pair | Entry Time | Exit Time | Duration |
+    |------|------------|-----------| -------- |
+    | BTC/USDT | 2024-01-01 10:00:00 | 2021-01-01 10:05:00 | 5m |
+    | ETH/USDT | 2024-01-01 10:05:00 | 2021-01-01 10:15:00 | 10m |
+    | XRP/USDT | 2024-01-01 10:15:00 | 2021-01-01 10:30:00 | 15m |
+    | SOL/USDT | 2024-01-01 10:15:00 | 2021-01-01 11:05:00 | 50m |
+    | BTC/USDT | 2024-01-01 11:05:00 | 2021-01-01 12:00:00 | 55m |
+
+    Without timeframe-detail, this would look like:
+
+    | Pair | Entry Time | Exit Time | Duration |
+    |------|------------|-----------| -------- |
+    | BTC/USDT | 2024-01-01 10:00:00 | 2021-01-01 11:00:00 | 1h |
+    | BTC/USDT | 2024-01-01 11:00:00 | 2021-01-01 12:00:00 | 1h |
+
+    The difference is significant, as without detail data, only the first `max_open_trades` signals per candle are evaluated, and the trade slots are only freed at the end of the candle, allowing for a new trade to be opened at the next candle.
+
+
 ## Backtesting multiple strategies
 
 To compare multiple strategies, a list of Strategies can be provided to backtesting.

freqtrade/optimize/backtesting.py

@@ -1378,28 +1378,6 @@ class Backtesting:
         current_time: datetime,
         trade_dir: LongShort | None,
         can_enter: bool,
-    ) -> None:
-        """
-        Conditionally call backtest_loop_inner a 2nd time if shorting is enabled,
-         a position closed and a new signal in the other direction is available.
-        """
-        if not self._can_short or trade_dir is None:
-            # No need to reverse position if shorting is disabled or there's no new signal
-            self.backtest_loop_inner(row, pair, current_time, trade_dir, can_enter)
-        else:
-            for _ in (0, 1):
-                a = self.backtest_loop_inner(row, pair, current_time, trade_dir, can_enter)
-                if not a or a == trade_dir:
-                    # the trade didn't close or position change is in the same direction
-                    break
-
-    def backtest_loop_inner(
-        self,
-        row: tuple,
-        pair: str,
-        current_time: datetime,
-        trade_dir: LongShort | None,
-        can_enter: bool,
     ) -> LongShort | None:
         """
         NOTE: This method is used by Hyperopt at each iteration. Please keep it optimized.
@@ -1429,7 +1407,7 @@ class Backtesting:
             and (self._position_stacking or len(LocalTrade.bt_trades_open_pp[pair]) == 0)
             and not PairLocks.is_pair_locked(pair, row[DATE_IDX], trade_dir)
         ):
-            if self.trade_slot_available(LocalTrade.bt_open_open_trade_count_candle):
+            if self.trade_slot_available(LocalTrade.bt_open_open_trade_count):
                 trade = self._enter_trade(pair, row, trade_dir)
                 if trade:
                     self.wallets.update()
@@ -1455,7 +1433,7 @@ class Backtesting:
             return exiting_dir
         return None
 
-    def get_detail_data(self, pair: str, row: tuple) -> DataFrame | None:
+    def get_detail_data(self, pair: str, row: tuple) -> list[tuple] | None:
         """
         Spread into detail data
         """
@@ -1474,44 +1452,143 @@ class Backtesting:
         detail_data.loc[:, "exit_short"] = row[ESHORT_IDX]
         detail_data.loc[:, "enter_tag"] = row[ENTER_TAG_IDX]
         detail_data.loc[:, "exit_tag"] = row[EXIT_TAG_IDX]
-        return detail_data
+        return detail_data[HEADERS].values.tolist()
 
-    def time_generator(self, start_date: datetime, end_date: datetime):
+    def _time_generator(self, start_date: datetime, end_date: datetime):
         current_time = start_date + self.timeframe_td
         while current_time <= end_date:
             yield current_time
             current_time += self.timeframe_td
 
+    def _time_generator_det(self, start_date: datetime, end_date: datetime):
+        """
+        Loop for each detail candle.
+        Yields only the start date if no detail timeframe is set.
+        """
+        if not self.timeframe_detail_td:
+            yield start_date, True, False, 0
+            return
+
+        current_time = start_date
+        i = 0
+        while current_time <= end_date:
+            yield current_time, i == 0, True, i
+            i += 1
+            current_time += self.timeframe_detail_td
+
+    def _time_pair_generator_det(self, current_time: datetime, pairs: list[str]):
+        for current_time_det, is_first, has_detail, idx in self._time_generator_det(
+            current_time, current_time + self.timeframe_td
+        ):
+            # Pairs that have open trades should be processed first
+            new_pairlist = list(dict.fromkeys([t.pair for t in LocalTrade.bt_trades_open] + pairs))
+            for pair in new_pairlist:
+                yield current_time_det, is_first, has_detail, idx, pair
+
     def time_pair_generator(
-        self, start_date: datetime, end_date: datetime, increment: timedelta, pairs: list[str]
+        self,
+        start_date: datetime,
+        end_date: datetime,
+        pairs: list[str],
+        data: dict[str, list[tuple]],
     ):
         """
         Backtest time and pair generator
-        :returns: generator of (current_time, pair, is_first)
-            where is_first is True for the first pair of each new candle
+        :returns: generator of (current_time, pair, row, is_last_row, trade_dir)
+            where is_last_row is a boolean indicating if this is the data end date.
         """
-        current_time = start_date + increment
+        current_time = start_date + self.timeframe_td
         self.progress.init_step(
             BacktestState.BACKTEST, int((end_date - start_date) / self.timeframe_td)
         )
-        for current_time in self.time_generator(start_date, end_date):
-            # Loop for each time point.
+        # Indexes per pair, so some pairs are allowed to have a missing start.
+        indexes: dict = defaultdict(int)
 
+        for current_time in self._time_generator(start_date, end_date):
+            # Loop for each main candle.
             self.check_abort()
             # Reset open trade count for this candle
             # Critical to avoid exceeding max_open_trades in backtesting
             # when timeframe-detail is used and trades close within the opening candle.
-            LocalTrade.bt_open_open_trade_count_candle = LocalTrade.bt_open_open_trade_count
             strategy_safe_wrapper(self.strategy.bot_loop_start, supress_error=True)(
                 current_time=current_time
             )
+            pair_detail_cache: dict[str, list[tuple]] = {}
+            pair_tradedir_cache: dict[str, LongShort | None] = {}
+            pairs_with_open_trades = [t.pair for t in LocalTrade.bt_trades_open]
+
+            for current_time_det, is_first, has_detail, idx, pair in self._time_pair_generator_det(
+                current_time, pairs
+            ):
+                # Loop for each detail candle (if necessary) and pair
+                # Yields only the main date if no detail timeframe is set.
 
                 # Pairs that have open trades should be processed first
-            new_pairlist = list(dict.fromkeys([t.pair for t in LocalTrade.bt_trades_open] + pairs))
+                trade_dir: LongShort | None = None
+                if is_first:
+                    # Main candle
+                    row_index = indexes[pair]
+                    row = self.validate_row(data, pair, row_index, current_time)
+                    if not row:
+                        continue
 
-            for pair in new_pairlist:
-                yield current_time, pair
+                    row_index += 1
+                    indexes[pair] = row_index
+                    is_last_row = current_time == end_date
+                    self.dataprovider._set_dataframe_max_index(self.required_startup + row_index)
+                    trade_dir = self.check_for_trade_entry(row)
+                    pair_tradedir_cache[pair] = trade_dir
 
+                else:
+                    # Detail candle - from cache.
+                    detail_data = pair_detail_cache.get(pair)
+                    if detail_data is None or len(detail_data) <= idx:
+                        # logger.info(f"skipping {pair}, {current_time_det}, {trade_dir}")
+                        continue
+                    row = detail_data[idx]
+                    trade_dir = pair_tradedir_cache.get(pair)
+
+                    if self.strategy.ignore_expired_candle(
+                        current_time - self.timeframe_td,  # last closed candle is 1 timeframe away.
+                        current_time_det,
+                        self.timeframe_secs,
+                        trade_dir is not None,
+                    ):
+                        # Ignore late entries eventually
+                        trade_dir = None
+
+                self.dataprovider._set_dataframe_max_date(current_time_det)
+
+                pair_has_open_trades = len(LocalTrade.bt_trades_open_pp[pair]) > 0
+                if pair in pairs_with_open_trades and not pair_has_open_trades:
+                    # Pair has had open trades which closed in the current main candle.
+                    # Skip this pair for this timeframe
+                    continue
+                if pair_has_open_trades and pair not in pairs_with_open_trades:
+                    # auto-lock for pairs that have open trades
+                    # Necessary for detail - to capture trades that open and close within
+                    # the same main candle
+                    pairs_with_open_trades.append(pair)
+
+                if (
+                    is_first
+                    and (trade_dir is not None or pair_has_open_trades)
+                    and has_detail
+                    and pair not in pair_detail_cache
+                    and pair in self.detail_data
+                    and row
+                ):
+                    # Spread candle into detail timeframe and cache that -
+                    # only once per main candle
+                    # and only if we can expect activity.
+                    pair_detail = self.get_detail_data(pair, row)
+                    if pair_detail is not None:
+                        pair_detail_cache[pair] = pair_detail
+                    row = pair_detail_cache[pair][idx]
+
+                is_last_row = current_time_det == end_date
+
+                yield current_time_det, pair, row, is_last_row, trade_dir
             self.progress.increment()
 
     def backtest(self, processed: dict, start_date: datetime, end_date: datetime) -> dict[str, Any]:
@@ -1535,60 +1612,26 @@ class Backtesting:
         # (looping lists is a lot faster than pandas DataFrames)
         data: dict = self._get_ohlcv_as_lists(processed)
 
-        # Indexes per pair, so some pairs are allowed to have a missing start.
-        indexes: dict = defaultdict(int)
-
         # Loop timerange and get candle for each pair at that point in time
-        for current_time, pair in self.time_pair_generator(
-            start_date, end_date, self.timeframe_td, list(data.keys())
-        ):
-            row_index = indexes[pair]
-            row = self.validate_row(data, pair, row_index, current_time)
-            if not row:
-                continue
-
-            row_index += 1
-            indexes[pair] = row_index
-            is_last_row = current_time == end_date
-            self.dataprovider._set_dataframe_max_index(self.required_startup + row_index)
-            self.dataprovider._set_dataframe_max_date(current_time)
-            trade_dir: LongShort | None = self.check_for_trade_entry(row)
-
-            pair_has_open_trades = len(LocalTrade.bt_trades_open_pp[pair]) > 0
-            if (
-                (trade_dir is not None or pair_has_open_trades)
-                and self.timeframe_detail
-                and pair in self.detail_data
-            ):
-                # Spread out into detail timeframe.
-                # Should only happen when we are either in a trade for this pair
-                # or when we got the signal for a new trade.
-                detail_data = self.get_detail_data(pair, row)
-
-                if detail_data is None or len(detail_data) == 0:
-                    # Fall back to "regular" data if no detail data was found for this candle
-                    self.dataprovider._set_dataframe_max_date(current_time)
-                    self.backtest_loop(row, pair, current_time, trade_dir, not is_last_row)
-                    continue
-                is_first = True
-                current_time_det = current_time
-                for det_row in detail_data[HEADERS].values.tolist():
-                    self.dataprovider._set_dataframe_max_date(current_time_det)
-                    self.backtest_loop(
-                        det_row,
+        for (
+            current_time,
             pair,
-                        current_time_det,
+            row,
+            is_last_row,
             trade_dir,
-                        is_first and not is_last_row,
-                    )
-                    current_time_det += self.timeframe_detail_td
-                    is_first = False
-                    if pair_has_open_trades and not len(LocalTrade.bt_trades_open_pp[pair]) > 0:
-                        # Auto-lock pair for the rest of the candle if the trade has been closed.
-                        break
-            else:
-                self.dataprovider._set_dataframe_max_date(current_time)
+        ) in self.time_pair_generator(start_date, end_date, list(data.keys()), data):
+            if not self._can_short or trade_dir is None:
+                # No need to reverse position if shorting is disabled or there's no new signal
                 self.backtest_loop(row, pair, current_time, trade_dir, not is_last_row)
+            else:
+                # Conditionally call backtest_loop a 2nd time if shorting is enabled,
+                # a position closed and a new signal in the other direction is available.
+
+                for _ in (0, 1):
+                    a = self.backtest_loop(row, pair, current_time, trade_dir, not is_last_row)
+                    if not a or a == trade_dir:
+                        # the trade didn't close or position change is in the same direction
+                        break
 
         self.handle_left_open(LocalTrade.bt_trades_open_pp, data=data)
         self.wallets.update()

freqtrade/persistence/trade_model.py

@@ -393,7 +393,6 @@ class LocalTrade:
     # Copy of trades_open - but indexed by pair
     bt_trades_open_pp: dict[str, list["LocalTrade"]] = defaultdict(list)
     bt_open_open_trade_count: int = 0
-    bt_open_open_trade_count_candle: int = 0
     bt_total_profit: float = 0
     realized_profit: float = 0
 
@@ -769,7 +768,6 @@ class LocalTrade:
         LocalTrade.bt_trades_open = []
         LocalTrade.bt_trades_open_pp = defaultdict(list)
         LocalTrade.bt_open_open_trade_count = 0
-        LocalTrade.bt_open_open_trade_count_candle = 0
         LocalTrade.bt_total_profit = 0
 
     def adjust_min_max_rates(self, current_price: float, current_price_low: float) -> None:
@@ -1471,11 +1469,6 @@ class LocalTrade:
         LocalTrade.bt_trades_open.remove(trade)
         LocalTrade.bt_trades_open_pp[trade.pair].remove(trade)
         LocalTrade.bt_open_open_trade_count -= 1
-        if (trade.close_date_utc - trade.open_date_utc) > timedelta(minutes=trade.timeframe):
-            # Only subtract trades that are open for more than 1 candle
-            # To avoid exceeding max_open_trades.
-            # Must be reset at the start of every candle during backesting.
-            LocalTrade.bt_open_open_trade_count_candle -= 1
         LocalTrade.bt_trades.append(trade)
         LocalTrade.bt_total_profit += trade.close_profit_abs
 
@@ -1485,7 +1478,6 @@ class LocalTrade:
             LocalTrade.bt_trades_open.append(trade)
             LocalTrade.bt_trades_open_pp[trade.pair].append(trade)
             LocalTrade.bt_open_open_trade_count += 1
-            LocalTrade.bt_open_open_trade_count_candle += 1
         else:
             LocalTrade.bt_trades.append(trade)
 
@@ -1494,9 +1486,6 @@ class LocalTrade:
         LocalTrade.bt_trades_open.remove(trade)
         LocalTrade.bt_trades_open_pp[trade.pair].remove(trade)
         LocalTrade.bt_open_open_trade_count -= 1
-        # TODO: The below may have odd behavior in case of canceled entries
-        # It might need to be removed so the trade "counts" as open for this candle.
-        LocalTrade.bt_open_open_trade_count_candle -= 1
 
     @staticmethod
     def get_open_trades() -> list[Any]:

tests/optimize/test_backtesting.py

@@ -870,6 +870,7 @@ def test_backtest_one_detail(default_conf_usdt, mocker, testdatadir, use_detail)
     backtesting = Backtesting(default_conf_usdt)
     backtesting._set_strategy(backtesting.strategylist[0])
     backtesting.strategy.populate_entry_trend = advise_entry
+    backtesting.strategy.ignore_buying_expired_candle_after = 59
     backtesting.strategy.custom_entry_price = custom_entry_price
     pair = "XRP/ETH"
     # Pick a timerange adapted to the pair we use to test
@@ -940,7 +941,7 @@ def test_backtest_one_detail(default_conf_usdt, mocker, testdatadir, use_detail)
 @pytest.mark.parametrize(
     "use_detail,exp_funding_fee, exp_ff_updates",
     [
-        (True, -0.018054162, 11),
+        (True, -0.018054162, 10),
         (False, -0.01780296, 6),
     ],
 )
@@ -1002,7 +1003,7 @@ def test_backtest_one_detail_futures(
     results = result["results"]
     assert not results.empty
     # Timeout settings from default_conf = entry: 10, exit: 30
-    assert len(results) == (5 if use_detail else 2)
+    assert len(results) == (4 if use_detail else 2)
 
     assert "orders" in results.columns
     data_pair = processed[pair]
@@ -1775,16 +1776,21 @@ def test_backtest_multi_pair_detail_simplified(
 
     if use_detail:
         # Backtest loop is called once per candle per pair
-        # Exact numbers depend on trade state - but should be around 3_800
-        assert bl_spy.call_count > 3_350
-        assert bl_spy.call_count < 3_800
+        # Exact numbers depend on trade state - but should be around 2_600
+        assert bl_spy.call_count > 2_170
+        assert bl_spy.call_count < 2_800
+        assert len(evaluate_result_multi(results["results"], "1h", 3)) > 0
     else:
         assert bl_spy.call_count < 995
+        assert len(evaluate_result_multi(results["results"], "1h", 3)) == 0
 
     # Make sure we have parallel trades
     assert len(evaluate_result_multi(results["results"], "1h", 2)) > 0
+    assert len(evaluate_result_multi(results["results"], "5m", 2)) > 0
     # make sure we don't have trades with more than configured max_open_trades
-    assert len(evaluate_result_multi(results["results"], "1h", 3)) == 0
+    # This must evaluate on detail timeframe - as we can have entries within the candle.
+    assert len(evaluate_result_multi(results["results"], "5m", 3)) == 0
+    assert len(evaluate_result_multi(results["results"], "1m", 3)) == 0
 
     # # Cached data correctly removed amounts
     offset = 1 if tres == 0 else 0
@@ -1803,7 +1809,12 @@ def test_backtest_multi_pair_detail_simplified(
         "end_date": max_date,
     }
     results = backtesting.backtest(**backtest_conf)
+    if use_detail:
+        assert len(evaluate_result_multi(results["results"], "1h", 1)) > 0
+    else:
         assert len(evaluate_result_multi(results["results"], "1h", 1)) == 0
+    assert len(evaluate_result_multi(results["results"], "5m", 1)) == 0
+    assert len(evaluate_result_multi(results["results"], "1m", 1)) == 0
 
 
 @pytest.mark.parametrize("use_detail", [True, False])
@@ -1900,9 +1911,9 @@ def test_backtest_multi_pair_long_short_switch(
 
     if use_detail:
         # Backtest loop is called once per candle per pair
-        assert bl_spy.call_count == 1484
+        assert bl_spy.call_count == 1511
     else:
-        assert bl_spy.call_count == 479
+        assert bl_spy.call_count == 508
 
     # Make sure we have parallel trades
     assert len(evaluate_result_multi(results["results"], "5m", 0)) > 0

tests/persistence/test_persistence.py

@@ -2145,7 +2145,6 @@ def test_Trade_object_idem():
         "bt_trades_open",
         "bt_trades_open_pp",
         "bt_open_open_trade_count",
-        "bt_open_open_trade_count_candle",
         "bt_total_profit",
         "from_json",
     )