polakowo / vectorbt

Find your trading edge, using the fastest engine for backtesting, algorithmic trading, and research.
https://vectorbt.dev
Other
4.36k stars 619 forks source link

Entry/Exit Dates Not Displaying Correctly on Plot for Trades w/ get_orders() SizeType = 'TargetPercent' #31

Closed kmcentush closed 4 years ago

kmcentush commented 4 years ago

Code

Here's some code to reproduce the issue:

# Get prices
prices = pd.DataFrame(np.array([[1, 1.1, 1.05, 1.11, 1.12], [1.03, 1.13, 1.07, 1.12, 1.13]]).T, columns=['BTC', 'ETH'])
prices.columns.name = 'asset'
prices.index.name = 'Date'

print(prices)

# Get order target percentages
cols = pd.MultiIndex.from_product([[1, 2], [2, 3], ['BTC', 'ETH']])
tgt_pct = pd.DataFrame(np.array([[0, 0.02, 0.03, 0, 0.05], [0.01, 0.03, 0.01, 0.02, 0.04],
                                [0, 0.04, 0.01, 0.02, 0.04], [0.03, 0.05, 0, 0.02, 0.03],
                                [0.01, 0.03, 0.01, 0.02, 0.04], [0, 0.04, 0.01, 0.02, 0.04],
                                [0.03, 0.05, 0, 0.02, 0.03], [0.01, 0.03, 0.01, 0.02, 0.04],
]).T, columns=cols)
tgt_pct.columns.names = ['custom_param1', 'custom_param2', 'asset']
tgt_pct.index.name = 'Date'

print(tgt_pct)

# Align prices
prices = prices.vbt.align_to(tgt_pct)

# Run the portfolio
size_type = getattr(vbt.portfolio.enums.SizeType, 'TargetPercent')
portfolio = vbt.Portfolio.from_orders(prices, tgt_pct, size_type=size_type, freq='1D')

# Plot a subset of the trades
portfolio.xs((2, 3, 'BTC'), axis=1).trades.plot().show_png()
portfolio.xs((2, 3, 'ETH'), axis=1).trades.plot().show_png()

Here are the plots produced by the snippet: image image

Issue

I'm not sure what the "proper" fix would look like for this problem, but all of the entry dates are being set to the date that the position was first held (even after closing an entire position by setting order target percent to 0). Additionally, it's not clear to me what determines an "exit" in this case. Dates where the order's target percentage is lower than the previous day is not always showing up as an exit. Ex: Date 2 for (2, 3, 'BTC') has no exit on the plot, but there is on Date 3. However, Date 2 is an exit for (2, 3, 'ETH'). Are the order target percentages for Date i being applied to the simulation on Date i, or Date i + 1? The above example makes me feel like it's both based on the plot results...

Apologies if anything above is confusing. I'm still trying to wrap my head around how the simulation takes order target percentages into effect.

Would there be any way to convey that additional/fewer holdings were taken in a position while showing the entry date as when a position changed? I guess this is more of a visualization problem than functionality problem. I believe that the sharpe, etc. are still correct for this scenario.

polakowo commented 4 years ago

There is definitely something going wrong here, but I cannot debug it because I’m on trip and not near my computer. What you can do is do print(portfolio.xs((2, 3, 'BTC'), axis=1).orders.records) and see where vectorbt placed orders. Plots like this mean the position hasn’t been closed, such that first buy is still an opening price for the sell orders to follow until the position is closed (when inside position, each sell takes average price of all previous buys and its opening index is the opening index of the current position). The percentage is applied on the same tick. I will try running this example today using Colab.

polakowo commented 4 years ago

Okay, after a bit of debugging with my phone I found where the bug is coming from: this line has to be removed: https://github.com/polakowo/vectorbt/blob/07866779173fe2da4b46ad784b5925ee37db2674/vectorbt/portfolio/nb.py#L184. For now, you can replace your size of 0 with -np.inf and it should work fine. If you want you can remove that line, increment the package version and make a pull request. Otherwise I can only upload the fix after Friday.

kmcentush commented 4 years ago

Just submit a PR. Let me know if I should change anything else. Thanks for looking into this!

polakowo commented 4 years ago

Thank you! The fix is now on PyPi.

Has this resolved your issue? Does the column (2, 3, ‘BTC’) now create two positions?

kmcentush commented 4 years ago

Looks like it fixed the issue for (2, 3, 'BTC'), yes. However, the (2, 3, 'ETH') plot is still a little funky in that all positions are the start date because no position was every fully closed. I'm also not sure why it has two entry points despite there being three instances where larger positions are taken. In any case, I'm still not sure what the best way to display incremental position changes would be...

image image

polakowo commented 4 years ago

I knew that such confusion would be coming. If you take a look at the documentation, there are two types of events in vectorbt: trades and positions. Positions have information on two types of events: when you entered the position and when you closed it. If you plot positions with portfolio.positions.plot() of the second column you will see that there is only one position, as you expected. Trades are more fine-grained: each trade corresponds to a sell operation. So, if you decreased your position gradually, it will display lots of small trades, such that you can analyze how good your exit strategy is. That’s why second plot shows you two trades. Both trades have opening index at the same time because their opening price is a size-weighted average over all purchase prices before them (within a position), so their opening index is always the opening index of the position. I will later introduce an example in the docs to make it more intuitive.

polakowo commented 4 years ago

I’m still not sure what the best way of plotting trades and positions would be, as I couldn’t find anything similar in any backtesting framework.

Edit: if you want a more basic plot such as displaying buy/sell markers, you can do portfolio.orders.plot(), and then portfolio.trades.pnl.to_mattix().vbt.scatter() to plot P&L of your trades.

kmcentush commented 4 years ago

In trying your to_matrix() idea for pnl, I ran into a weird bug that is causing two trades to occur on the same (last) date index in some instances. Here's a .ipynb that illustrates the issue.I'm not sure how to create a reproducible example with the made-up code snippets, but I've attached a more complete notebook that demonstrates the issue.

Link: https://github.com/polakowo/vectorbt/files/5031987/crypto_orders.zip

polakowo commented 4 years ago

It’s not a bug. Similar to positions, trades can also be open. If at the last tick the position is still open, it will calculate a trade needed to close the position. Bit since at this last tick you have another trade that still doesn’t close the position, you have two trades pointing at the same tick. This is also pointed out in the docs - you can’t transform records to matrix then if you have conflicts like that. What you can do is to choose which kind of trades to use: portfolio.trades.open or portfolio.trades.closed.

kmcentush commented 4 years ago

Ah, that makes sense. Apologies on my end for not looking into the docs enough to understand that. I presume the portfolio statistics only reflect closed trades then, no?

On Thu, Aug 6, 2020, 12:12 AM Oleg Polakow notifications@github.com wrote:

It’s not a bug. Similar to positions, trades can also be open. If at the last tick the position is still open, it will calculate a trade needed to close the position. Bit since at this last tick you have another trade that still doesn’t close the position, you have two trades pointing at the same tick. This is also pointed out in the docs - you can’t transform records to matrix then if you have conflicts like that. What you can do is to choose which kind of trades to use: portfolio.trades.open or portfolio.trades.closed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/polakowo/vectorbt/issues/31#issuecomment-669750378, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE5SCMBG3M6HEGD35G5NSILR7JJV7ANCNFSM4PSZDL7A .

polakowo commented 4 years ago

Portfolio stats reflect both open and closed trades, because reducing operations are applied on records in a map-reduce manner without conversion to matrix form, thus they don’t care about position conflicts. This was actually the reason why I moved from matrix to record representation of event data, because this way we can store multiple elements under the same tick. This will also allow us in future versions of vectorbt to make multiple orders per tick.

Generally, if you look how records were implemented, you will see that some attributes return filtered records. Take for example positions: operations on portfolio.positions will take into account all positions, portfolio.positions.closed only closed, portfolio.positions.closed.winning only closed positions with positive P&L, etc. You can even define your own filters. Portfolio returns stats on trades/positions without any filters, thus on all events.

kmcentush commented 4 years ago

I see. The logic makes sense, especially considering the future when you can have multiple orders in the same tick. However, could it seemingly be dangerous to include open positions in portfolio stats? There's no guarantee that an order will go through on a future date, so it could skew the results. In my case, the open order will have a return of 18% or something. That's not insignificant.

Do other backtesting libraries do similar things in terms of portfolio statistics w/ both open and closed positions?

The filtering of records makes sense, thanks.

polakowo commented 4 years ago

That’s a good argument, I never thought about that in depth. The reason why I added this is because if you buy and hold you will still be able to see this position in stats, otherwise all will be NaN. What can be done is adding a keyword argument when initializing portfolio, let’s say ‘unrealized_stats’, that when True will include open positions, otherwise only closed. This can be False by default. But this won’t affect the design of records, ‘portfolio.positions’ and ‘portfolio.trades’ must still return both open and closed events, since when plotting them user must see that his position might not be closed. Maybe we can include an additional warning in docs that records without filters may return unrealized metrics and user must explicitly query ‘closed’ attribute to exclude them.

kmcentush commented 4 years ago

I think it would be good to include a toggle for the portfolio statistics to include unrealized as you said. Or at the very least, enable a way to calculate them efficiently on any filtered trades object (or however you normally do it).

polakowo commented 4 years ago

Added the toggle in 0.13.4.

polakowo commented 4 years ago

I have looked at the implementation in other backtesting libs, and it seems most of them return statistics of the closed trades only (as you suggested) but calculate unrealized metrics the same way as vectorbt does: using the price at the last tick and without any commissions and slippages applied (thus shouldn't be used in stats, only for information purposes). Thus having the toggle is the most appropriate solution.

kmcentush commented 4 years ago

The implementation looks good to me, especially defaulting to realized-only statistics.

I'm closing this issue for now, I believe you've adequately addressed any issues with plotting multiple orders/explained why things are the case and alternative ways to view the equivalent information.