search

nautechsystems / nautilus_trader

A high-performance algorithmic trading platform and event-driven backtester
https://nautilustrader.io
GNU Lesser General Public License v3.0
2.18k stars 498 forks source link

Overstated PnL on position flip #1083

Closed cjdsellers closed 1 year ago

cjdsellers commented 1 year ago

Bug Report

Reported from @Scout in Discord.

Is it possible that there is something wrong with the pnl calculation when a position is flipped? It seems like the pnl calculation is overstated when flipping a position.

To test this I broke the flip orders (e.g. -1 to + 0.5) into a close position order (-1 to 0) + new second order (0 to 0.5), and with this approach I pnl results that match my own independent backtest. [4:10 AM] I think the pnl is overstated by the amount flipped e.g. if your position at (T) is -1, and your position at (T+1) is +0.5, it seems to think that the PNL at (T+1) is 1.5 * the price delta rather than just 1. [4:15 AM] As an example - this is the result of flipping a position, you can notice that the pnl goes from 1_000_034.71457617 to 1_000_148.44323617, even though the realised return is 88.68245000

2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: Updated AccountState(account_id=BINANCE-001, account_type=MARGIN, base_currency=None, is_reported=False, balances=[AccountBalance(total=1_000_034.71457617 USDT, locked=499.97765950 USDT, free=999_534.73691667 USDT)], margins=[MarginBalance(initial=0.00000000 USDT, maintenance=249.97815950 USDT, instrument_id=XLMUSDT-PERP.BINANCE), MarginBalance(initial=0.00000000 USDT, maintenance=249.99950000 USDT, instrument_id=XMRUSDT-PERP.BINANCE)], event_id=aab7571f-4178-4ef4-a8a6-00cb365f58ca).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.PairTrader-000: <--[EVT] OrderFilled(instrument_id=XLMUSDT-PERP.BINANCE, client_order_id=O-20220801-001-000-17, venue_order_id=BINANCE-1-009, account_id=BINANCE-001, trade_id=BINANCE-1-017, position_id=left-0, order_side=BUY, order_type=MARKET, last_qty=490, last_px=0.11831 USDT, commission=0.00000000 USDT, liquidity_side=TAKER, ts_event=1659344400088000000).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: XLMUSDT-PERP.BINANCE net_position=-83332.0
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: Updated AccountState(account_id=BINANCE-001, account_type=MARGIN, base_currency=None, is_reported=False, balances=[AccountBalance(total=1_000_034.71457617 USDT, locked=498.51635700 USDT, free=999_536.19821917 USDT)], margins=[MarginBalance(initial=0.00000000 USDT, maintenance=248.51685700 USDT, instrument_id=XLMUSDT-PERP.BINANCE), MarginBalance(initial=0.00000000 USDT, maintenance=249.99950000 USDT, instrument_id=XMRUSDT-PERP.BINANCE)], event_id=6b0ebb44-1afe-45a6-86cd-10ddfa2fc6cd).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.PairTrader-000: <--[EVT] PositionChanged(instrument_id=XLMUSDT-PERP.BINANCE, position_id=left-0, account_id=BINANCE-001, opening_order_id=O-20220801-001-000-1, closing_order_id=None, entry=SELL, side=SHORT, signed_qty=-83332.0, quantity=83_332, peak_qty=150_451, currency=USDT, avg_px_open=0.11929086980647949, avg_px_close=0.11912080305858488, realized_return=0.00143, realized_pnl=7.77792729 USDT, unrealized_pnl=81.73784271 USDT, ts_opened=1659315602716000000, ts_last=1659344400088000000, ts_closed=0, duration_ns=0).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.PairTrader-000: Position(SHORT 83_332 XLMUSDT-PERP.BINANCE, id=left-0)
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: Updated AccountState(account_id=BINANCE-001, account_type=MARGIN, base_currency=None, is_reported=False, balances=[AccountBalance(total=1_000_148.44323617 USDT, locked=498.51635700 USDT, free=999_649.92687917 USDT)], margins=[MarginBalance(initial=0.00000000 USDT, maintenance=248.51685700 USDT, instrument_id=XLMUSDT-PERP.BINANCE), MarginBalance(initial=0.00000000 USDT, maintenance=249.99950000 USDT, instrument_id=XMRUSDT-PERP.BINANCE)], event_id=6611a42e-f407-4031-b346-4062884811dd).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.PairTrader-000: <--[EVT] OrderFilled(instrument_id=XLMUSDT-PERP.BINANCE, client_order_id=O-20220801-001-000-17, venue_order_id=BINANCE-1-009, account_id=BINANCE-001, trade_id=BINANCE-1-018, position_id=left-0, order_side=BUY, order_type=MARKET, last_qty=117_141, last_px=0.11832 USDT, commission=0.00000000 USDT, liquidity_side=TAKER, ts_event=1659344400088000000).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: XLMUSDT-PERP.BINANCE net_position=0.0
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: Updated AccountState(account_id=BINANCE-001, account_type=MARGIN, base_currency=None, is_reported=False, balances=[AccountBalance(total=1_000_148.44323617 USDT, locked=249.99950000 USDT, free=999_898.44373617 USDT)], margins=[MarginBalance(initial=0.00000000 USDT, maintenance=249.99950000 USDT, instrument_id=XMRUSDT-PERP.BINANCE)], event_id=0d19007c-f3f4-466d-ab1b-b703bd36c471).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.PairTrader-000: <--[EVT] PositionClosed(instrument_id=XLMUSDT-PERP.BINANCE, position_id=left-0, account_id=BINANCE-001, opening_order_id=O-20220801-001-000-1, closing_order_id=O-20220801-001-000-17, entry=SELL, side=FLAT, signed_qty=0.0, quantity=0, peak_qty=150_451, currency=USDT, avg_px_open=0.11929086980647949, avg_px_close=0.11885381423913263, realized_return=0.00366, realized_pnl=88.68245000 USDT, unrealized_pnl=0.00000000 USDT, ts_opened=1659315602716000000, ts_last=1659344400088000000, ts_closed=1659344400088000000, duration_ns=28797372000000).
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: XLMUSDT-PERP.BINANCE net_position=33809.0
2022-08-01T09:00:00.088000000Z [INF] BACKTESTER-001.Portfolio: Updated AccountState(account_id=BINANCE-001, account_type=MARGIN, base_currency=None, is_reported=False, balances=[AccountBalance(total=1_000_148.44323617 USDT, locked=350.00652200 USDT, free=999_798.43671417 USDT)], margins=[MarginBalance(initial=0.00000000 USDT, maintenance=249.99950000 USDT, instrument_id=XMRUSDT-PERP.BINANCE), MarginBalance(initial=0.00000000 USDT, maintenance=100.00702200 USDT, instrument_id=XLMUSDT-PERP.BINANCE)], event_id=1891ca80-4017-4090-8e50-be5d9934e1cd).

Expected Behavior

Realized PnL in this scenario should be 88.68245000

Actual Behavior

Realized PnL was 113.7286600000225 (overstated)

cjdsellers commented 1 year ago

This has been confirmed and now fixed on develop from d140d9eb4520766dafe53e8bbd15bbc94744b5e5.

The issue was that when flipping the position - the entire filled quantity was being booked for realized PnL, rather than the positions current open quantity. So the fixes were along the lines of:

position.pyx

    cdef double _calculate_pnl(
        self,
        double avg_px_open,
        double avg_px_close,
        double quantity,
    ):
        # Only book open quantity towards PnL
        quantity = fmin(quantity, fabs(self.signed_qty))  # <-----------------------

        if self.is_inverse:
            # In base currency
            return quantity * self.multiplier.as_f64_c() * self._calculate_points_inverse(avg_px_open, avg_px_close)
        else:
            # In quote currency
            return quantity * self.multiplier.as_f64_c() * self._calculate_points(avg_px_open, avg_px_close)

And also during cash account calculations:

        cdef double fill_qty = fill.last_qty.as_f64_c()
        cdef double fill_px = fill.last_px.as_f64_c()

        if position is not None:
            # Only book open quantity towards realized PnL
            fill_qty = fmin(fill_qty, position.quantity.as_f64_c())