BUG: Fix max drawdown calculation.

The input into max drawdown was incorrect, causing the bad results.
i.e. the `compounded_log_returns` were not values representative of
the algorithms total return at a given time, though
`calculate_max_drawdown` was treating the values as if they were.
Instead, use the `algorithm_period_returns` series, which does provide
the total return.

Update risk answer key with an Excel calculation of max drawdown
to help corroborate the calculations.

Also, remove `compounded_log_returns`, (which actually had stopped
being the `compounded_log_returns` at some point), since the max
drawdown was the only calculation using the values in that series.

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))),
 })

tests/risk/risk-answer-key-checksums

@@ -11,3 +11,4 @@ cc507b6fca18aabadac69657181edd4e
 75c1b1441efbc2431215835a5079ccc6
 37e3ea4a1788f1aa6f3ee0986bc625ae
 651e611e723e2a58b1ded91d0cd39b66
+d62fce39ec78f032165d8f356bba5c2c

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,))

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