-
-
Notifications
You must be signed in to change notification settings - Fork 847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible Calculation Error in CAGR Calculation #291
Comments
252 days is very commonly used in finance for trading. |
I think zntech is correct. If our resurns senquence has date index, we should use 365, if our returns sequence has no index, then we use 252. |
@zntech is right. When calculating CAGR, you are using calendar year of 365 days (see https://www.investopedia.com/terms/c/cagr.asp#:~:text=To%20calculate%20the%20CAGR%20of,one%20from%20the%20subsequent%20result.) Using 252 periods is wrong and will produce misleading CAGR. For a simple example: I created #302 to fix it |
this is the same than this PR the fix is really simple. and already implemented in https://github.com/gnzsnz/quantstats-cagr/tree/cagr if you need a quick fix you can do
|
The tool is very useful, but there might be an error in the calculated CAGR. Your algorithm is based on subtracting the start and end dates of the sequence using calendar days. However, you're using 252 trading days per year to calculate the interval length, which leads to an incorrect number of years due to the following calculation using the variable "periods" (which, based on your function's default parameters, is set to 252):
years = (returns.index[-1] - returns.index[0]).days / periods
I believe the correct approach should involve directly calculating the interval length in terms of calendar days to determine the number of years:
years = (returns.index[-1] - returns.index[0]).days / 365
The text was updated successfully, but these errors were encountered: