santoshlite / Empyrial

An Open Source Portfolio Backtesting Engine for Everyone | 面向所有人的开源投资组合回测引擎
https://empyrial.gitbook.io/empyrial/
MIT License
914 stars 124 forks source link

Is get_returns correct for getting portfolio return? #104

Closed pzzhang closed 1 year ago

pzzhang commented 1 year ago

Describe the bug In https://github.com/ssantoshp/Empyrial/blob/main/src/empyrial/main.py#L122-L127, we compute the return of a portfolio as below. if len(stocks) > 1: ret_data = assets.pct_change()[1:] returns = (ret_data * wts).sum(axis=1)

Is this correct?

Expected behavior As the time changes, the weights of stocks in the portfolio are changing. Should wts be updated when computing the return?

santoshlite commented 1 year ago

Hey, what are you trying to achieve? You can use rebalancing to keep the same weights over time. Are you referring to something similar to #77? I just noticed it and will work on that!

pzzhang commented 1 year ago

@ssantoshp , yes, I'm referring to the same issue to #77.

I have a fix in this diff: https://github.com/pzzhang/Empyrial/commit/1ac72904c4111ae3724427eca7f2b20047db8489

santoshlite commented 1 year ago

Oh my god, you're fantastic! It would have taken me some time to get back and remember how we designed this (I am not the one who implemented rebalancing). Would you like to make a pull request? I don't mind copying the file either, up to you.

santoshlite commented 1 year ago

Fixed