BUG: fix annualization of alpha #60
Conversation
|
This looks right. But would hope that some tests catch this we need to change. |
|
@twiecki just updated |
| @@ -1111,7 +1114,7 @@ def test_roll_up_capture(self, returns, factor_returns, window, expected): | |||
| (empty_returns, simple_benchmark, (np.nan, np.nan)), | |||
| (one_return, one_return, (np.nan, np.nan)), | |||
| (mixed_returns[1:], negative_returns[1:], | |||
| (-8.306666666666668, -0.71296296296296313)), | |||
| (-0.9997853834885004, -0.71296296296296313)), | |||
There was a problem hiding this comment.
This already shows why the previous result was wrong.
|
LGTM. We should probably alert others to this bugfix. |
empyrical/stats.py
Outdated
| @@ -818,7 +818,7 @@ def alpha_aligned(returns, factor_returns, risk_free=0.0, period=DAILY, | |||
| adj_factor_returns = _adjust_returns(factor_returns, risk_free) | |||
| alpha_series = adj_returns - (_beta * adj_factor_returns) | |||
|
|
|||
| return nanmean(alpha_series) * ann_factor | |||
| return (nanmean(alpha_series) + 1) ** ann_factor - 1 | |||
There was a problem hiding this comment.
What would you think about breaking this formula out into a helper function? Feels like that would make it clearer what's happening here, and less error prone.
(At the least a comment to the effect of #59 would be good)
There was a problem hiding this comment.
added an annualize helper in utils, and used that here
empyrical/utils.py
Outdated
| @@ -443,3 +443,10 @@ def default_returns_func(symbol, start=None, end=None): | |||
| rets = get_symbol_returns_from_yahoo(symbol, start=start, end=end) | |||
|
|
|||
| return rets[symbol] | |||
|
|
|||
|
|
|||
| def annualize(stat, ann_factor=252): | |||
There was a problem hiding this comment.
Should this be called something like annualize_compounding? From what I understand the compounding is why this formula applies?
There was a problem hiding this comment.
👍 renamed to annualize_compounding
|
We are good to release |
|
LGTM |
|
Can we merge this? |
|
We chose to wait for when we have the bandwidth to write a blog post announcing the change. Will follow up on that today |
|
I think there is still a bug here, as the failing tests show. Seems like it always returns 0 or nan? |
|
Thanks @gmanoim-quantopian ! |
Fixes incorrect alpha calculation: quantopian/empyrical#60
Fixes incorrect alpha calculation: quantopian/empyrical#60
For #59, annualize alpha correctly