From 82d2ddfa9051b3bd0e9c7425b08eb544834c49d9 Mon Sep 17 00:00:00 2001 From: Brian Fink Date: Tue, 14 Apr 2015 12:36:26 -0400 Subject: [PATCH] BUG: Fix limit orders Only fill limit order if impacted fill price is better than the limit price. If a limit order is partially filled, only fill the remaining shares if the impacted fill price is better than the limit price. --- tests/finance/test_slippage.py | 99 ++++++++++++++++++++++++++++++++-- zipline/finance/slippage.py | 18 +++++-- 2 files changed, 109 insertions(+), 8 deletions(-) diff --git a/tests/finance/test_slippage.py b/tests/finance/test_slippage.py index 403d8b46..d0aa10cf 100644 --- a/tests/finance/test_slippage.py +++ b/tests/finance/test_slippage.py @@ -103,12 +103,13 @@ class SlippageTestCase(TestCase): ] orders_txns = list(slippage_model.simulate( - events[2], + events[3], open_orders )) self.assertEquals(len(orders_txns), 0) - # long, does trade + # long, does not trade - impacted price worse than limit price + open_orders = [ Order(**{ 'dt': datetime.datetime(2006, 1, 5, 14, 30, tzinfo=pytz.utc), @@ -123,6 +124,24 @@ class SlippageTestCase(TestCase): open_orders )) + self.assertEquals(len(orders_txns), 0) + + # long, does trade + + open_orders = [ + Order(**{ + 'dt': datetime.datetime(2006, 1, 5, 14, 30, tzinfo=pytz.utc), + 'amount': 100, + 'filled': 0, + 'sid': 133, + 'limit': 3.6}) + ] + + orders_txns = list(slippage_model.simulate( + events[3], + open_orders + )) + self.assertEquals(len(orders_txns), 1) txn = orders_txns[0][1] @@ -160,7 +179,7 @@ class SlippageTestCase(TestCase): self.assertEquals(len(orders_txns), 0) - # short, does trade + # short, does not trade - impacted price worse than limit price open_orders = [ Order(**{ @@ -176,6 +195,24 @@ class SlippageTestCase(TestCase): open_orders )) + self.assertEquals(len(orders_txns), 0) + + # short, does trade + + open_orders = [ + Order(**{ + 'dt': datetime.datetime(2006, 1, 5, 14, 30, tzinfo=pytz.utc), + 'amount': -100, + 'filled': 0, + 'sid': 133, + 'limit': 3.4}) + ] + + orders_txns = list(slippage_model.simulate( + events[1], + open_orders + )) + self.assertEquals(len(orders_txns), 1) _, txn = orders_txns[0] @@ -372,7 +409,7 @@ class SlippageTestCase(TestCase): self.assertEquals(len(orders_txns), 0) - # long, does trade + # long, does not trade - impacted price worse than limit price open_orders = [ Order(**{ @@ -396,6 +433,32 @@ class SlippageTestCase(TestCase): open_orders )) + self.assertEquals(len(orders_txns), 0) + + # long, does trade + + open_orders = [ + Order(**{ + 'dt': datetime.datetime(2006, 1, 5, 14, 30, tzinfo=pytz.utc), + 'amount': 100, + 'filled': 0, + 'sid': 133, + 'stop': 4.0, + 'limit': 3.6}) + ] + + orders_txns = list(slippage_model.simulate( + events[2], + open_orders + )) + + self.assertEquals(len(orders_txns), 0) + + orders_txns = list(slippage_model.simulate( + events[3], + open_orders + )) + self.assertEquals(len(orders_txns), 1) _, txn = orders_txns[0] @@ -436,7 +499,7 @@ class SlippageTestCase(TestCase): self.assertEquals(len(orders_txns), 0) - # short, does trade + # short, does not trade - impacted price worse than limit price open_orders = [ Order(**{ @@ -460,6 +523,32 @@ class SlippageTestCase(TestCase): open_orders )) + self.assertEquals(len(orders_txns), 0) + + # short, does trade + + open_orders = [ + Order(**{ + 'dt': datetime.datetime(2006, 1, 5, 14, 30, tzinfo=pytz.utc), + 'amount': -100, + 'filled': 0, + 'sid': 133, + 'stop': 3.0, + 'limit': 3.4}) + ] + + orders_txns = list(slippage_model.simulate( + events[0], + open_orders + )) + + self.assertEquals(len(orders_txns), 0) + + orders_txns = list(slippage_model.simulate( + events[1], + open_orders + )) + self.assertEquals(len(orders_txns), 1) _, txn = orders_txns[0] diff --git a/zipline/finance/slippage.py b/zipline/finance/slippage.py index 64f1ebd6..772d9e85 100644 --- a/zipline/finance/slippage.py +++ b/zipline/finance/slippage.py @@ -258,13 +258,25 @@ class VolumeShareSlippage(SlippageModel): simulated_impact = volume_share ** 2 \ * math.copysign(self.price_impact, order.direction) \ * event.price + impacted_price = event.price + simulated_impact + + if order.limit: + # this is tricky! if an order with a limit price has reached + # the limit price, we will try to fill the order. do not fill + # these shares if the impacted price is worse than the limit + # price. return early to avoid creating the transaction. + + # buy order is worse if the impacted price is greater than + # the limit price. sell order is worse if the impacted price + # is less than the limit price + if (order.direction > 0 and impacted_price > order.limit) or \ + (order.direction < 0 and impacted_price < order.limit): + return return create_transaction( event, order, - # In the future, we may want to change the next line - # for limit pricing - event.price + simulated_impact, + impacted_price, math.copysign(cur_volume, order.direction) )