Position direction unchanged after executing an order of size greater than position size in the opposite direction

[scoriiu]

Position unrealized PNL with wrong value after executing an order of size grater than position size in the opposite direction.

I opened a PR with a test illustrating this: https://github.com/nautechsystems/nautilus_trader/pull/59

I'm not sure what the expected behaviour should be in this case, I'm thinking that flattening the current position and opening a new one in the opposite direction should be the right approach. Another option is to keep the same position and reset the impacted fields, but then the realized_pnl will not contain the PNL prior to changing the direction.

[scoriiu]

On a separate note, I suggest renaming the structure MarketPosition to something more explicit like PositionSide

[cjdsellers]

I had a play around on the testnet. I agree if a position is 'overfilled' or flipped intentionally by the trader then the entry direction should change (this has been fixed now).

Although, I think it's cleaner to close out the original direction and then enter a new one. Allows a clearer audit trail.

[cjdsellers]

On a separate note, I suggest renaming the structure MarketPosition to something more explicit like PositionSide

Yes, that could be a better name because it does refer to the side of a position only really. I'll have a think about the rename.

[cjdsellers]

Ok, I agree with you. PositionSide is much more explicit, has some nice naming symmetry with OrderSide. I think market position was just something I picked up from NinjaTrader over 5 years ago - no good reason to keep that enum name. Definitely give me all your opinions on naming as there as some things I'm just too used to seeing.

[scoriiu]

I also think that closing the original position is a cleaner solution, otherwise like you say, the information from the original position will be lost when generating the positions report at the end.

[scoriiu]

Regarding the test, I don't think the way you've changed it is correct:

        self.assertEqual(position.unrealized_points(reduce_quote), -10.0)
        self.assertEqual(position.unrealized_pnl(reduce_quote).as_double(), -500000.0)

Immediately after flipping the position side, the unrealized PNL should be 0. This will be fixed if a new position will be created instead.

[cjdsellers]

Ok I'll have another look at it unless you're able to propose a fix?

[scoriiu]

I will have a look and open a PR. Shall the desired behaviour be that the original position is closed and a new one be created? The new position will have a new ID as well.

[cjdsellers]

Thinking about it that's the most logical way forward, we should keep things clean. I'm actually considering dispensing with the concept of a ClientPositionId, it's just one more moving part. It would simplify the ExecutionEngine too. If the broker/exchange assigns a position identifier then that will be used, otherwise one will be generated in a logical way keeping track of the total position count per symbol (including what's been cached in the database). So there will just be the PositionId.

One consideration though is how different strategies will handle trading the same instrument on the same account with netting order management (it may not be possible), we may have to just keep positions consistent with what's represented at the exchange, so I have to keep thinking about it and draw some diagrams.

[scoriiu]

I think this would simplify things. In my experience I didn't encounter the possibility of having multiple positions for the same instrument, on the same direction on the same broker. Most of the exchanges allow only one position per instrument. Few of them (e.g. OkEx) allow a position for LONGs and a position for SHORTs per instrument. Personally I prefer having one position per instrument per broker.

[scoriiu]

In this case I will wait until your work is done and then propose a fix for this.