nautechsystems / nautilus_trader

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

IB positions/orders not syncing in NautilusTrader when orders are placed externally #1190

Closed dkharrat closed 1 year ago

dkharrat commented 1 year ago

When an order is placed externally or is externally modified (e.g. via IB's Trader Workstation), the list of positions/orders are not updated in NautilusTrader.

Expected Behavior

The expectation is that positions are synced so that the strategy can check existing positions to make decisions.

Actual Behavior

Positions are not synced with NautilusTrader.

Steps to Reproduce the Problem

  1. Place a buy order using IB's Trader Workstation.
  2. Start a live trading session using NautilusTrader. Notice at this stage, positions and orders are correctly synced.
  3. Place a sell order using IB's Trader Workstation.
  4. Issue: NautilusTrader is unaware of the updated order and position.

Specifications

rsmb7z commented 1 year ago

Hi @dkharrat It is because the way IB API works, the client_id (default 1 in Nautilus) used to connect can only see its own orders. Having said that if you have multiple instance of Nauilus or any other client app using different client_id, so each of the client_id will be able to see its own orders only. This applies only to Orders, and all client's can see the Open Positions. Your strategy should be setup accordingly to claim those positions, however even if not claimed, you should see the open positions in Nautilus Cache. Also if any order is created using Nautilus Strategy and then you modify or cancel the order manually in TWS - it will back propogate to the Nautilus and is the tested scenario.

There is special way where you can claim orders created using Workstation, but that also means that it would claim any orders from other client_id's as well. This can be achieved by setting client_id=0 or working with master_id. This scenario is not specifically tested and hence missing any error handling arrising due to this.

dkharrat commented 1 year ago

Thanks for the explanation! Client id behavior makes sense. However, the issue is that since external orders and positions are synced in Nautilus in step 2, any future changes to their state (both positions and orders) is never updated. This means, Nautilus will eventually have stale positions/orders in its cache, which is error prone if any strategy logic relies knowing the state of these positions.

I believe if all orders/positions are synced in step 2, they should be updated to reflect their true state for correctness. Otherwise, if the intention is to only consider positions/orders initiated by the strategy, external positions/orders should be ignored in step 2.

Please let me know if I'm missing something.

dkharrat commented 1 year ago

Also, I just noticed that this issue occurs even for orders generated by Nautilus. Steps that reproduce it:

  1. Start live trading session using NautilusTrader and allow it to initiate a limit order (such that order does not execute immediately).
  2. Stop the live trading process (to simulate the process crashing, for example).
  3. Restart the live trading session.
  4. Issue: When Nautilus syncs the orders and positions, they are marked as "external" and are no longer associated with the strategy. This means any logic that relies on the strategy's open positions/orders will no longer work as expected.
  5. Another Issue: Once the order is executed, the order/position is not synced in Nautilus, resulting in stale data.

Update: I also see this error in the logs after restarting the live trading session in step 3:

report.avg_px was `None` when a value was expected.

cjdsellers commented 1 year ago

Hi @dkharrat

It's not well documented right now, but there are a couple of config options which can deal with external orders for these sorts of use cases.

https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/config/live.py#L73 https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/config/common.py#L435

I hope this helps clear some things up, please follow up with any further thoughts though!

rsmb7z commented 1 year ago
  • Issue: When Nautilus syncs the orders and positions, they are marked as "external" and are no longer associated with the strategy. This means any logic that relies on the strategy's open positions/orders will no longer work as expected.
  • Another Issue: Once the order is executed, the order/position is not synced in Nautilus, resulting in stale data.

Once you configuer external_order_claims as mentioned by @cjdsellers, excution will be handled as expected.

dkharrat commented 1 year ago

Thanks @cjdsellers and @rsmb7z. This is very helpful. Unfortunately, I still see a few issues:

  1. I tried using external_order_claims, and it did tie the "external" position to the strategy when Nautilus starts up. However, when I manually closed it via TWS, Nautilus did not detect that the position was closed. It seems that syncing only happens during the Nautilus startup. If any position is opened/closed while Nautilus is running, it's not synced, even with external_order_claims set. I believe IBAPI does send position info to clients, so this should be possible to fix.
  2. If a position is already open in IB and if I do not set external_order_claims, Nautilus sometimes fails to start up. I noticed it only happens when there's an open bracket order in IB. I see the following error in the logs:
    2023-08-15T09:31:41.254803486Z [ERR] TRADER-001.ExecEngine: report.avg_px was `None` when a value was expected.
    2023-08-15T09:31:41.254938598Z [ERR] TRADER-001.TradingNode: Execution state could not be reconciled.

    This can be reproduced using the following steps: a. Start a live trading session using NautilusTrader. b. Have the strategy trigger a bracket order (market + TP + SL) c. Once the market order is executed, stop NautilusTrader d. Start NautilusTrader again e. Issue: error occurs and NautilusTrader fails to start up.

Syncing positions is particularly important for reliability and robustness to handle failure cases (e.g. NautilusTrader crashing/restarting, etc.)

rsmb7z commented 1 year ago
  1. I tried using external_order_claims, and it did tie the "external" position to the strategy when Nautilus starts up. However, when I manually closed it via TWS, Nautilus did not detect that the position was closed. It seems that syncing only happens during the Nautilus startup. If any position is opened/closed while Nautilus is running, it's not synced, even with external_order_claims set. I believe IBAPI does send position info to clients, so this should be possible to fix.

Unfortunately such updates are not sent to API Client on fly, the reason as I explained early is because to close position you create new order and any actions related to Orders [not by the API client itself] are not sent out to API. However we do have OnDemand access to open positions irrespective of opened by any client or TWS which are requested using this method. I cannot recall if generate_position_status_reports is continously called by ExecEngine. @cjdsellers is there built-in way that if position is not returned from adapter but is opened in Nautilus, does it Close within node?

2. If a position is already open in IB and if I do not set external_order_claims, Nautilus sometimes fails to start up. I noticed it only happens when there's an open bracket order in IB. I see the following error in the logs:

I understood the problem, will review and fix. With only SL shouldn't occur.

dkharrat commented 1 year ago

However we do have OnDemand access to open positions irrespective of opened by any client or TWS which are requested using this method. I cannot recall if generate_position_status_reports is continously called by ExecEngine. @cjdsellers is there built-in way that if position is not returned from adapter but is opened in Nautilus, does it Close within node?

That's a good approach, which should work. I noticed that IBAPI sends account updates whenever a position is opened/closed, so this could perhaps be used as a way to trigger reading the positions on-demand and update it in Nautilus if there were any changes.

I understood the problem, will review and fix. With only SL shouldn't occur.

Great, thanks for looking into it!

rsmb7z commented 1 year ago

2. If a position is already open in IB and if I do not set external_order_claims, Nautilus sometimes fails to start up. I noticed it only happens when there's an open bracket order in IB. I see the following error in the logs:

@dkharrat this has been fixed in PR.

  1. I tried using external_order_claims, and it did tie the "external" position to the strategy when Nautilus starts up. However, when I manually closed it via TWS, Nautilus did not detect that the position was closed. It seems that syncing only happens during the Nautilus startup. If any position is opened/closed while Nautilus is running, it's not synced, even with external_order_claims set. I believe IBAPI does send position info to clients, so this should be possible to fix.

For this the suitable solution I suggest for you is using client_id=0 for connection which is the designed way by IB where you want the API to claim external orders so we don't loose the indtended purpose of of multiple client_ids where isolation is the purpose and ideally you shouldn't be interfering with strategies (same instruments) to avoid unexpected consequences.

@cjdsellers I have tested and implemented the placeholder where we will receive the Order during runtime when using client_id=0. The straight-forward way I found for these orders to infer from OrderStatusReport so rest of sequence would be followed as-is, however I see that it is called by Engine itself but we will need similar behaviour from within client to achieve this. Leaving this to you if can be implemented or you would prefer to postpone for later stage.

dkharrat commented 1 year ago

thanks for implementing a solution @rsmb7z! Does this work for open orders too? for example, if the strategy created a bracket order and then Nautilus restarts (due to crash), does it sync the open orders so that the strategy can continue where it left off?

rsmb7z commented 1 year ago

@dkharrat yes it should recover the open positions and open orders (as long were created using same client-id) and that is the primary use case expectation for me as well. Just in case you find any specific order type not working feel free to report and I would be happy to fix. You can do the crash testing on paper account to be sure it works well in your specific use case.

cjdsellers commented 1 year ago

Were there any further actionables for this one @dkharrat and @rsmb7z ?

rsmb7z commented 1 year ago

@cjdsellers the bug was fixed and @dkharrat may confirm if solution worked for him. There is another point from him, more of additional feature which I can accomodate if there is way to to publish OrderStatusReport from DataClient to DataEngine because as far I understand current implementation is in-response when requested by DataEngine.

cjdsellers commented 1 year ago

@cjdsellers the bug was fixed and @dkharrat may confirm if solution worked for him. There is another point from him, more of additional feature which I can accomodate if there is way to to publish OrderStatusReport from DataClient to DataEngine because as far I understand current implementation is in-response when requested by DataEngine.

Did you mean ExecutionClient -> ExecutionEngine? looks like the reports are requested by the LiveExecutionEngine as part of the reconciliation routine right now.

rsmb7z commented 1 year ago

Did you mean ExecutionClient -> ExecutionEngine? looks like the reports are requested by the LiveExecutionEngine as part of the reconciliation routine right now.

Yes you are right, I meant ExecutionClient -> ExecutionEngine. Sync inconsistency while thinking and typing :thinking_face: So this would make possible external_order_claims from broker to the Nautilus in runtime.

cjdsellers commented 1 year ago

So we're wanting the ability to request an OrderStatusReport for any client_order_id at runtime?

What you want might already be available via the strategy query_order(order) method, which will eventually result in an OrderStatusReport being returned (or error if the exchange doesn't recognize the ID). The LiveExecutionEngine will then automatically carry out any necessary reconciliation.

You mentioned external order claims though, so maybe there is more to this?

rsmb7z commented 1 year ago

@cjdsellers The current flow is that Actor or ExecEngine make request for query_order (when Nautilus need update for order known by it or when it request the orders on startup) and ExecClient in response send them OrderStatusReport. However the case in-question, is for the Order which is not initiated by Nautilus while it was running,

  1. Order initiated from the Web Client or TWS client
  2. Order notification received at ExecClient
  3. There is no path to update back to ExecEngine (unlike on start where same this is done but in response to ExecEngine request)

Btw I know this may require some changes at ExecEngine and I am okay if you prefer to do after rust migration. This is indeed not bug and more of new feature whereas current assumption is that there shall be no interference for the instrument for which Nautilus Strategy is running, from third-party i.e. Web Client or another API client.

cjdsellers commented 1 year ago

Hi @rsmb7z

I think this case is handled for Binance though, when an external order (such as one placed through the web UI) becomes an OrderStatusReport and makes it back to the LiveExecutionEngine, its processed here and if not found in the cache will then generate an external order.

cjdsellers commented 1 year ago

Hi @rsmb7z

So in the BinanceFuturesExecutionClient in the order trade update handler, we attempt to pull the matching strategy ID for the order out of the cache. If the order is not previously registered with a strategy then we assume it's an external order and send an OrderStatusReport back to the LiveExecutionEngine for reconciliation.

We could probably follow the same pattern for the IB adapter?

https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/adapters/binance/futures/schemas/user.py#L282

    def handle_order_trade_update(  # noqa: C901 (too complex)
        self,
        exec_client: BinanceCommonExecutionClient,
    ) -> None:
        """
        Handle BinanceFuturesOrderData as payload of ORDER_TRADE_UPDATE event.
        """
        client_order_id = ClientOrderId(self.c) if self.c != "" else None
        ts_event = millis_to_nanos(self.T)
        venue_order_id = VenueOrderId(str(self.i))
        instrument_id = exec_client._get_cached_instrument_id(self.s)
        strategy_id = exec_client._cache.strategy_id_for_order(client_order_id)  # <---
        if strategy_id is None:
            report = self.parse_to_order_status_report(
                account_id=exec_client.account_id,
                instrument_id=instrument_id,
                client_order_id=client_order_id,
                venue_order_id=venue_order_id,
                ts_event=ts_event,
                ts_init=exec_client._clock.timestamp_ns(),
                enum_parser=exec_client._enum_parser,
            )
            exec_client._send_order_status_report(report)
cjdsellers commented 1 year ago

Closing this for now as we could open a more specific issue for this if we needed to, thanks everyone for the follow ups here! :pray:

rsmb7z commented 1 year ago
exec_client._send_order_status_report(report)

@cjdsellers this is what was required indeed, for some reason I over looked. I will include this in the next PR.