goldmansachs / gs-quant

Python toolkit for quantitative finance
https://developer.gs.com/discover/products/gs-quant/
Apache License 2.0
6k stars 767 forks source link

PricingContext._handle_results incorrectly removes unfetched Futures #82

Open AlexanderJHall opened 4 years ago

AlexanderJHall commented 4 years ago

PricingContext._handle_results incorrectly removes unfetched Futures

PricingContext in markets.core

The following code (with dummy details) should return the ir delta of the swap and the PV of the bond. - however it throws a KeyError

with dev_session:        
        with PricingContext(is_batch=False):
            irs = IRSwap(PayReceive.Receive,'5y', Currency.GBP, 10000000, fixed_rate=0.05)
            irs_delta = irs.calc(IRDelta)
            bond = Security(isin='XS0123456789')
            bond_price = bond.price()

        print('irs delta')
        print(irs_delta.result())
        print('bond price')
        print(bond_price.result())

However the code fails with the following error:

    self._calc()
  File gs_quant\markets\core.py, line 237, in _calc
    run_request(risk_request, GsSession.current)
  File gs_quant\markets\core.py, line 201, in run_request
    self._handle_results(calc_result)
  File gs_quant\markets\core.py, line 256, in _handle_results
    positions_for_measure = self.__futures[risk_measure]
KeyError: Price

Explanation: Two Futures are created in PricingContext.futures as {IRDelta: Future1, Price: Future2}. The results are fetched sequentially from the Session. While processing the first returned results, the IRDelta, PricingContext._handle_results correctly removes the first future {IRDelta:Future1} from Futures and sets the IRDelta onto the Future (via .set_results) - all this happens in lines 249 to 260 However _handle_results assumes that all remaining Futures are bad (see comment on line 262) and removes them from futures (lines 264 to 268). As a result when the second set of results, bond price, come back from the Session and flow through _handle_results futures is an empty dictionary and so the bond price can't be bound to a future (as the future was destroyed while processing the previous result)

andrewphillipsn commented 4 years ago

thanks @AlexanderJHall, we'll take a look. @nick-young-gs @david-vanden-bon can one of you guys check this out pls

@AlexanderJHall also happy to take an MR :)