annual_returns calculation for 'calender' style is not accurate

[justinlent]

I don't believe it computes the average annual return correctly since it currently just divides by num_years, where it should be computing the geometric return along the lines of:

total_return = (portfolio_end_value - portfolio_start_value) / portfolio_start_value
annual_return = (1+total_return) ^ (1/num_years) - 1

@twiecki Thoughts?

[justinlent]

which is just the CAGR (compounded annual growth rate) calculation, so maybe we should change the method name from calendar -> CAGR. Looks like folks in the Q forums refer to this method using CAGR. As well the annual return from CAGR is the same as "compound" which I somewhat proved to myself in an Excel spreadsheet, so having this style may just be redundant unless I'm missing an edge case or something.

[twiecki]

We're on the same page and should fix that asap.

[justinlent]

my current working opinion is we should simplify annual_return to simply be CAGR, so change the signature from:

def annual_return(returns, style='compound', period=DAILY)

to
def annual_return(returns, period=DAILY)

and then only have it return the CAGR result which is the fixed version of 'calendar' as described earlier in this thread.

I checked and none of the other downstream performance stats depend on annual_return, since stats like Sharpe Ratio have been refactored to compute the returns in such a fashion as needed locally for themselves

[twiecki]

I agree with that.

[twiecki]

Assigning to you as you worked most closely with this.

[justinlent]

@twiecki can you do a quick code review of my implementation in this PR #234

[twiecki]

Closed via #234