nautechsystems / nautilus_trader

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

Error in order.apply() and solution suggestion: 'Filled' order marked as 'Partially filled' . #317

Closed ian-wazowski closed 3 years ago

ian-wazowski commented 3 years ago
2021-05-09T16:43:58.882574Z [INF] XEMM-002.SchmittHysteresisMarketMaking-001: [CMD]--> CancelOrder(client_id=BYBIT, trader_id=XEMM-002, account_id=BYBIT-001, instrument_id=BTC/USDT.BYBIT, client_order_id=O-20210509-164353-002-001-125, venue_order_id=08525bc5-78a6-45c0-8822-cfb7214b9fb4, command_id=948e2e57-89bf-ebfe-492a-441418ed30ba).
2021-05-09T16:43:58.887271Z [INF] XEMM-002.SchmittHysteresisMarketMaking-001: <--[EVT] OrderPendingCancel(account_id=BYBIT-001, client_order_id=O-20210509-164353-002-001-125, pending_ns=1620578638882885800, event_id=f749da18-07f7-3c52-9e74-2b05a785a5e9).
2021-05-09T16:43:59.002074Z [ERR] XEMM-002.CCXTExecClient-BYBIT: Theoretical leaves_qty(0.00035980) does not matched with actual leaves_qty(0.00000000).message: {'symbol': 'BTCUSDT', 'side': 'Buy', 'order_id': '08525bc5-78a6-45c0-8822-cfb7214b9fb4', 'exec_id': '2cf23c21-5652-5a37-a39a-a4e338a2bade', 'order_link_id': 'O-20210509-164353-002-001-125', 'price': 57577.496, 'order_qty': 0.009, 'exec_type': 'Trade', 'exec_qty': 0.009, 'exec_fee': -0.12954937, 'leaves_qty': 0, 'is_maker': True, 'trade_time': '2021-05-09T16:43:57.473428Z'}
2021-05-09T16:43:59.022283Z [ERR] XEMM-002.ExecEngine: InvalidStateTrigger(PENDING_CANCEL -> PARTIALLY_FILLED)
 Traceback (most recent call last):
  File "nautilus_trader/execution/engine.pyx", line 713, in nautilus_trader.execution.engine.ExecutionEngine._handle_order_event
    order.apply(event)
  File "nautilus_trader/model/orders/base.pyx", line 539, in nautilus_trader.model.orders.base.Order.apply
    self._fsm.trigger(OrderState.PARTIALLY_FILLED)
  File "nautilus_trader/core/fsm.pyx", line 117, in nautilus_trader.core.fsm.FiniteStateMachine.trigger
    raise InvalidStateTrigger(f"{self.state_string_c()} -> {self._trigger_parser(trigger)}")

2021-05-09T16:43:59.023175Z [WRN] XEMM-002.CCXTExecClient-BYBIT: ExchangeError: bybit {"ret_code":130032,"ret_msg":"order_status[Filled] cannot cancel","ext_code":"","ext_info":"","result":null,"time_now":"1620578637.520886","rate_limit_status":89,"rate_limit_reset_ms":1620578637519,"rate_limit":100} in _cancel_order

As you can see, the exchange said the order was exactly filled but Order.apply(event) did not indicate that it was filled.

I think there is floating point issue(we adapted Decimal Class massively as you know). Every time we add, subtract or converting type(float to double etc), we get a very small floating point error(a number less than 1e-10 or more), and because of this small decimal point, orders that should be marked as 'filled' are makerd as 'partial filled'.

>>> Decimal(1.0000) + Decimal(0.005)
Decimal('1.005000000000000000104083409')

>>> Decimal(1.0000) - Decimal(0.005)
Decimal('0.9949999999999999998959165914')

To solve this, I would like to suggest adding a very small(no effect on PnL) margin( It is to set the lower limit mathematically.) when checking the order quantity as shown below It seems to happen inevitably when trading forex.

# model/orders/base.pyx:531L
        elif isinstance(event, OrderFilled):
            # Check identifiers
            if self.venue_order_id.is_null():
                self.venue_order_id = event.venue_order_id
            else:
                Condition.not_in(event.execution_id, self._execution_ids, "event.execution_id", "self._execution_ids")
            # Fill order

            if self.filled_qty + event.last_qty + Decimal(1e-8) < self.quantity: # Give tiny margin to solve the floating point error
                self._fsm.trigger(OrderState.PARTIALLY_FILLED)
            else:
                self._fsm.trigger(OrderState.FILLED)
            self._filled(event)

        # Update events last as FSM may raise InvalidStateTrigger
        self._events.append(event)

Any thoughts?

cjdsellers commented 3 years ago

Thank you for raising this!

I saw you noticed on a previous commit I fixed the error in the order FSM to allow PENDING_CANCEL -> PARTIALLY_FILLED.

Could you please provide a code snippet where you assign the quantity for the order?

cjdsellers commented 3 years ago

I think there is a loophole in the value object validation where a user can pass any value str or Decimal into a Price or Quantity without a precision specified - which will result in something like Decimal('1.005000000000000000104083409').

Even if this is truncated on the exchange side to 1.0050 etc, the quantity remaining will still be 000000000000000000104083409 after all fills have come back from the exchange.

The two options we have to solve this from what I can see are either:

ian-wazowski commented 3 years ago

Thank you for raising this!

I saw you noticed on a previous commit I fixed the error in the order FSM to allow PENDING_CANCEL -> PARTIALLY_FILLED.

Could you please provide a code snippet where you assign the quantity for the order?

Thank you! Excellent work!

ian-wazowski commented 3 years ago

I think there is a loophole in the value object validation where a user can pass any value str or Decimal into a Price or Quantity without a precision specified - which will result in something like Decimal('1.005000000000000000104083409').

Even if this is truncated on the exchange side to 1.0050 etc, the quantity remaining will still be 000000000000000000104083409 after all fills have come back from the exchange.

The two options we have to solve this from what I can see are either:

  • Your above suggestion.
  • Requiring a precision be passed into every value object regardless of the type for the value (currently a precision is only enforced for floating point types).

Entering precision for all quantities seems to be a more accurate and predictable solution for this solution. When developing adapters, I have to take a closer look. Thank you.

cjdsellers commented 3 years ago

I'm leaning towards making precision a required argument for Price and Quantity although will have more of a think and consult some others. As you say it leads to more predictable outcomes, and doesn't rely on a user passing in a valid string or decimal value rounded to the correct precision for the instrument.

It also avoids this sort of epsilon comparison which is more appropriate for floating point arithmetic.

Attaching this link for an extreme yet readable deep dive into the subject too 😀

https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html

I welcome any further thoughts from yourself or others.

cjdsellers commented 3 years ago

The precision is now explicit as a required argument for Price and Quantity types, so this bug should be resolved.

See release notes for 1.119.0 for more details.