diff --git a/tests/risk/answer_key.py b/tests/risk/answer_key.py index 8ef2fccb..75c71509 100644 --- a/tests/risk/answer_key.py +++ b/tests/risk/answer_key.py @@ -249,6 +249,9 @@ class AnswerKey(object): 'CUMULATIVE_ALPHA': DataIndex( 'Sim Cumulative', 'AC', 4, 254), + 'CUMULATIVE_MAX_DRAWDOWN': DataIndex( + 'Sim Cumulative', 'AF', 4, 254), + } def __init__(self): @@ -319,4 +322,6 @@ RISK_CUMULATIVE = pd.DataFrame({ DATES, ANSWER_KEY.CUMULATIVE_ALPHA))), 'beta': pd.Series(dict(zip( DATES, ANSWER_KEY.CUMULATIVE_BETA))), + 'max_drawdown': pd.Series(dict(zip( + DATES, ANSWER_KEY.CUMULATIVE_MAX_DRAWDOWN))), }) diff --git a/tests/risk/risk-answer-key-checksums b/tests/risk/risk-answer-key-checksums index a43fec17..61a76cda 100644 --- a/tests/risk/risk-answer-key-checksums +++ b/tests/risk/risk-answer-key-checksums @@ -11,3 +11,4 @@ cc507b6fca18aabadac69657181edd4e 75c1b1441efbc2431215835a5079ccc6 37e3ea4a1788f1aa6f3ee0986bc625ae 651e611e723e2a58b1ded91d0cd39b66 +d62fce39ec78f032165d8f356bba5c2c diff --git a/tests/risk/test_risk_cumulative.py b/tests/risk/test_risk_cumulative.py index 16fb3a9a..1f3fc569 100644 --- a/tests/risk/test_risk_cumulative.py +++ b/tests/risk/test_risk_cumulative.py @@ -111,8 +111,10 @@ class TestRisk(unittest.TestCase): decimal=2, err_msg="Mismatch at %s" % (dt,)) - def test_max_drawdown_calculated(self): - # We don't track max_drawdown by day, so it doesn't make sense to - # generate a full answer key for it. For now, ensure it's just - # "not zero" - self.assertNotEqual(self.cumulative_metrics_06.max_drawdown, 0.0) + def test_max_drawdown_06(self): + for dt, value in answer_key.RISK_CUMULATIVE.max_drawdown.iterkv(): + np.testing.assert_almost_equal( + self.cumulative_metrics_06.max_drawdowns[dt], + value, + decimal=2, + err_msg="Mismatch at %s" % (dt,)) diff --git a/zipline/finance/risk/cumulative.py b/zipline/finance/risk/cumulative.py index 576db84c..c4beb9d0 100644 --- a/zipline/finance/risk/cumulative.py +++ b/zipline/finance/risk/cumulative.py @@ -190,7 +190,6 @@ class RiskMetricsCumulative(object): self.mean_benchmark_returns = None self.annualized_benchmark_returns = None - self.compounded_log_returns = pd.Series(index=cont_index) self.algorithm_period_returns = pd.Series(index=cont_index) self.benchmark_period_returns = pd.Series(index=cont_index) self.excess_returns = pd.Series(index=cont_index) @@ -267,8 +266,6 @@ class RiskMetricsCumulative(object): self.num_trading_days = len(self.algorithm_returns) - self.update_compounded_log_returns() - self.algorithm_period_returns[dt] = \ self.calculate_period_returns(self.algorithm_returns) self.benchmark_period_returns[dt] = \ @@ -379,39 +376,30 @@ algorithm_returns ({algo_count}) in range {start} : {end} on {dt}" return '\n'.join(statements) - def update_compounded_log_returns(self): - if len(self.algorithm_returns) == 0: - return - - try: - compound = math.log(1 + self.algorithm_returns[ - self.algorithm_returns.last_valid_index()]) - except ValueError: - compound = 0.0 - # BUG? Shouldn't this be set to log(1.0 + 0) ? - - if np.isnan(self.compounded_log_returns[self.latest_dt]): - self.compounded_log_returns[self.latest_dt] = compound - else: - self.compounded_log_returns[self.latest_dt] = \ - self.compounded_log_returns[self.latest_dt] + compound - def calculate_period_returns(self, returns): return (1. + returns).prod() - 1 def update_current_max(self): - if len(self.compounded_log_returns) == 0: + if len(self.algorithm_period_returns) == 0: return - if self.current_max < self.compounded_log_returns[self.latest_dt]: - self.current_max = self.compounded_log_returns[self.latest_dt] + if self.current_max < self.algorithm_period_returns[self.latest_dt]: + self.current_max = self.algorithm_period_returns[self.latest_dt] def calculate_max_drawdown(self): - if len(self.compounded_log_returns) == 0: + if len(self.algorithm_period_returns) == 0: return self.max_drawdown - cur_drawdown = 1.0 - math.exp( - self.compounded_log_returns[self.latest_dt] - - self.current_max) + # The drawdown is defined as: (high - low) / high + # The above factors out to: 1.0 - (low / high) + # + # Instead of explicitly always using the low, use the current total + # return value, and test that against the max drawdown, which will + # exceed the previous max_drawdown iff the current return is lower than + # the previous low in the current drawdown window. + cur_drawdown = 1.0 - ( + (1.0 + self.algorithm_period_returns[self.latest_dt]) + / + (1.0 + self.current_max)) self.drawdowns[self.latest_dt] = cur_drawdown