search

nautechsystems / nautilus_trader

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

Wrong account balance when MARKET order trigger multiple fills #1769

Closed Otlk closed 1 day ago

Otlk commented 3 days ago

Bug Report

Account balance is wrong when opening a position from a market order and multiple fills are created.

Expected Behavior

Should correctly compute fill and update account balance. Here the 2nd fill should have an impact of 7993 * 1.1501 ~ 9 192.75 USD

Actual Behavior

When submitting a market order that generate multiple fills, the wrong quantity is used to determine impact of further fills. https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/accounting/accounts/cash.pyx#L338:L357

In the following logs we see that 2 fills are created but when nautilus calculate pnl for the 2nd fill, the last quantity is used instead of the new one.

2022-12-02T14:33:00.000000000Z [INFO] BACKTESTER-001.Strat: [CMD]--> SubmitOrder(order=MarketOrder(BUY 8_261 HUT.XNAS MARKET GTC, status=INITIALIZED, client_order_id=O-20221202-1431-001-000-1, venue_order_id=None, position_id=None, tags=None), position_id=None)
2022-12-02T14:33:00.000000000Z [WARN] BACKTESTER-001.RiskEngine: Cannot check MARKET order risk: no prices for HUT.XNAS
2022-12-02T14:33:00.000000000Z [INFO] BACKTESTER-001.Strat: <--[EVT] OrderSubmitted(instrument_id=HUT.XNAS, client_order_id=O-20221202-1431-001-000-1, account_id=XNAS-001, ts_event=1669991460000000000)
2022-12-02T14:34:00.000000000Z [DEBUG] BACKTESTER-001.OrderMatchingEngine(XNAS): Applying fills to MarketOrder(BUY 8_261 HUT.XNAS MARKET GTC, status=SUBMITTED, client_order_id=O-20221202-1431-001-000-1, venue_order_id=None, position_id=None, tags=None), venue_position_id=None, position=None, fills=[(Price(1.1500), Quantity(268))]
2022-12-02T14:34:00.000000000Z [DEBUG] BACKTESTER-001.Cache: Indexed ClientOrderId('O-20221202-1431-001-000-1') with VenueOrderId('XNAS-1-001')
2022-12-02T14:34:00.000000000Z [DEBUG] BACKTESTER-001.Portfolio: Calculated PnLs: [Money(-308.20, USD)]
2022-12-02T14:34:00.000000000Z [INFO] BACKTESTER-001.Portfolio: Updated AccountState(account_id=XNAS-001, account_type=CASH, base_currency=USD, is_reported=False, balances=[AccountBalance(total=9_691.80 USD, locked=0.00 USD, free=9_691.80 USD)], margins=[], event_id=be6cb57b-4464-4f70-b412-bbea68fc7dd3)
2022-12-02T14:34:00.000000000Z [DEBUG] BACKTESTER-001.Portfolio: Updated OrderFilled(instrument_id=HUT.XNAS, client_order_id=O-20221202-1431-001-000-1, venue_order_id=XNAS-1-001, account_id=XNAS-001, trade_id=XNAS-1-003, position_id=HUT.XNAS-Strat-000, order_side=BUY, order_type=MARKET, last_qty=268, last_px=1.1500 USD, commission=0.00 USD, liquidity_side=TAKER, ts_event=1669991460000000000)
2022-12-02T14:34:00.000000000Z [INFO] BACKTESTER-001.Strat: <--[EVT] OrderFilled(instrument_id=HUT.XNAS, client_order_id=O-20221202-1431-001-000-1, venue_order_id=XNAS-1-001, account_id=XNAS-001, trade_id=XNAS-1-003, position_id=HUT.XNAS-Strat-000, order_side=BUY, order_type=MARKET, last_qty=268, last_px=1.1500 USD, commission=0.00 USD, liquidity_side=TAKER, ts_event=1669991460000000000)
2022-12-02T14:34:00.000000000Z [DEBUG] BACKTESTER-001.Cache: Indexed PositionId('HUT.XNAS-Strat-000'), client_order_id=O-20221202-1431-001-000-1, strategy_id=Strat-000)
2022-12-02T14:34:00.000000000Z [DEBUG] BACKTESTER-001.Cache: Added Position(id=HUT.XNAS-Strat-000, strategy_id=Strat-000)
2022-12-02T14:34:00.000000000Z [INFO] BACKTESTER-001.Portfolio: HUT.XNAS net_position=268
2022-12-02T14:35:00.000000000Z [INFO] BACKTESTER-001.Strat: <--[EVT] PositionOpened(instrument_id=HUT.XNAS, position_id=HUT.XNAS-Strat-000, account_id=XNAS-001, opening_order_id=O-20221202-1431-001-000-1, closing_order_id=None, entry=BUY, side=LONG, signed_qty=268.0, quantity=268, peak_qty=268, currency=USD, avg_px_open=1.15, avg_px_close=0.0, realized_return=0.00000, realized_pnl=0.00 USD, unrealized_pnl=0.00 USD, ts_opened=1669991460000000000, ts_last=1669991460000000000, ts_closed=0, duration_ns=0)
2022-12-02T14:35:00.000000000Z [DEBUG] BACKTESTER-001.Portfolio: Calculated PnLs: [Money(-308.23, USD)]
2022-12-02T14:35:00.000000000Z [INFO] BACKTESTER-001.Portfolio: Updated AccountState(account_id=XNAS-001, account_type=CASH, base_currency=USD, is_reported=False, balances=[AccountBalance(total=9_383.57 USD, locked=0.00 USD, free=9_383.57 USD)], margins=[], event_id=3554507c-3cfb-4275-a1a8-86a1c3716c36)
2022-12-02T14:35:00.000000000Z [DEBUG] BACKTESTER-001.Portfolio: Updated OrderFilled(instrument_id=HUT.XNAS, client_order_id=O-20221202-1431-001-000-1, venue_order_id=XNAS-1-001, account_id=XNAS-001, trade_id=XNAS-1-004, position_id=HUT.XNAS-Strat-000, order_side=BUY, order_type=MARKET, last_qty=7_993, last_px=1.1501 USD, commission=0.00 USD, liquidity_side=TAKER, ts_event=1669991460000000000)
2022-12-02T14:35:00.000000000Z [INFO] BACKTESTER-001.Strat: <--[EVT] OrderFilled(instrument_id=HUT.XNAS, client_order_id=O-20221202-1431-001-000-1, venue_order_id=XNAS-1-001, account_id=XNAS-001, trade_id=XNAS-1-004, position_id=HUT.XNAS-Strat-000, order_side=BUY, order_type=MARKET, last_qty=7_993, last_px=1.1501 USD, commission=0.00 USD, liquidity_side=TAKER, ts_event=1669991460000000000)
2022-12-02T14:35:00.000000000Z [INFO] BACKTESTER-001.Portfolio: HUT.XNAS net_position=8261
2022-12-02T14:35:00.000000000Z [INFO] BACKTESTER-001.Strat: <--[EVT] PositionChanged(instrument_id=HUT.XNAS, position_id=HUT.XNAS-Strat-000, account_id=XNAS-001, opening_order_id=O-20221202-1431-001-000-1, closing_order_id=None, entry=BUY, side=LONG, signed_qty=8261.0, quantity=8_261, peak_qty=8_261, currency=USD, avg_px_open=1.1500967558406974, avg_px_close=0.0, realized_return=0.00000, realized_pnl=0.00 USD, unrealized_pnl=0.03 USD, ts_opened=1669991460000000000, ts_last=1669991460000000000, ts_closed=0, duration_ns=0)

If needed can provide a MRE, thanks !

Steps to Reproduce the Problem

  1. Create a buy market order that generate multiple fills (order size > volume)

Specifications

cjdsellers commented 1 day ago

Hey @Otlk

Thanks for the report here.

Just to check on expectations for behavior, we don't expect any realized PnL if we're adding to a position - only when reducing. With this in mind do you still think there's a bug in the PnL calculation?

At what point did the -308.23 of PnL occur prior to these logs? It looks like a position was already open but then I see the PositionOpened event.

Also, is this instrument really quoted in USD to 4 decimal places?

Given you've identified where the PnL is calculated, do you have any suggestions for a fix at this point?

Otlk commented 1 day ago

Hi @cjdsellers,

With this in mind do you still think there's a bug in the PnL calculation?

The problem is that since calculate_pnls is called on open we could hit this condition on multiple fills, resulting in the wrong fill value:

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

This value is then used to incorrectly update account balances: I.e. from my logs above:

fill 1
Portfolio: Calculated PnLs: [Money(-308.20, USD)]  ## = 268 * 1.15
Portfolio: Updated AccountState(... balances=[AccountBalance(total=9_691.80 USD, locked=0.00 USD, free=9_691.80 USD)]...)
Portfolio: Updated OrderFilled(...last_qty=268, last_px=1.1500 USD ...)
Portfolio: HUT.XNAS net_position=268

Account balance is correct, 10000 - (268 * 1.15) = 9_691.80 USD

fill 2
Portfolio: Calculated PnLs: [Money(-308.23, USD)]  ## = 268 * 1.1501
Portfolio: Updated AccountState(...balances=[AccountBalance(total=9_383.57 USD, locked=0.00 USD, free=9_383.57 USD)]...)
Portfolio: Updated OrderFilled(...last_qty=7_993, last_px=1.1501 USD...)
Portfolio: HUT.XNAS net_position=8261

Account balance is wrong since we hit the condition: 9_691.80 - (7_993 * 1.1501) != 9_383.57 USD

Given you've identified where the PnL is calculated, do you have any suggestions for a fix at this point?

I think just checking that order side != position side is enough here ?

Also, is this instrument really quoted in USD to 4 decimal places?

Sub penny quoting is prohibited for stocks > 1$ on exchange but it occurs off exchange in ATS or wherever

Otlk commented 1 day ago

@cjdsellers Changing the account type from CASH to MARGIN produce the correct output. Doing the same as this in the CASH account should fix it. Underlying cause is that the calculate pnl function in the cash account dont calculate pnl directly but via the fill "notional" value that is then added or substracted to the account balance.

cjdsellers commented 1 day ago

Thanks @Otlk I see the issue now.

Fixed here af93bf1e497b427848e69c1eeaddba136916a453.