QuantLib allow swaptions to take OvernightIndexedSwap

closes #1588 I don't want to take any credit this for this code. This is cherry picked from https://github.com/OpenSourceRisk/QuantLib. @pcaspers mentionned before that he would be willing to contribute this to QuantLib, see https://github.com/lballabio/QuantLib/pull/1379#issuecomment-1123571248

Feb 08 '23 16:02 thrasibule

I have no objections doing this. Quite the contrary, it helps reducing the diff between our fork and the official repo!

Feb 08 '23 19:02 pcaspers

coverage: 72.497% (+0.07%) from 72.426% when pulling 9b4efa33c4cf21e503ff45a5acef79ef4fea3ee3 on thrasibule:ois_swaption2 into c6492d00290d55d1113fddde10997c617ce401c4 on lballabio:master.

Feb 08 '23 19:02 coveralls

Hi @thrasibule,

IMHO this PR looks a little bit like this anti-pattern:

We are now having both swap_ and swapOis_ in Swaption and later on having (a lot of) switches to decide if the underlying swap is a VanillaSwap or an OvernightIndexedSwap.

E.g. what about the other pricing engines in ql/pricingengines/swaption? Do they still work as designed if you pass in an OvernightIndexedSwap?

@pcaspers: I guess in ORE you don't care about the other engines aka don't use them?

Wouldn't it be better to derive an OvernightIndexedSwaption from Swaption and do the needed adjustments aka switch- and if -statements there?

Regards Ralf

Feb 09 '23 11:02 ralfkonrad

Yes, I was thinking about reworking this somehow. I'll give it a go.

Feb 09 '23 11:02 lballabio

Yes - the goal in ORE was to get it working with as little effort as possible.

Feb 09 '23 11:02 pcaspers

I think I'd duplicate the class so that we have a Swaption and an OvernightIndexedSwaption. Same for the Black engine. We can find later ways to factor out common code (templates, maybe? Types are different).

Also, I don't think any of the other engines (besides the Black engine in this PR) will work with an OI swaption — they all use arguments_.swap one way or the other. I think this also prevents the swaption helper from working with OI, as it can calculate the Black price but not the model price. @pcaspers, did you modify those as well in ORE?

Mar 10 '23 11:03 lballabio

Yes we have adapted those as well

Mar 10 '23 17:03 pcaspers

The danger here being that a partial migration and / or refactoring will cause conflicts in our codebase. Not sure if this is the right way forward?

Mar 10 '23 18:03 pcaspers

Hmm. Yes, we probably need to look at those as well. Do you have additional engines in QuantExt too?

Mar 14 '23 13:03 lballabio

Another road here would be to revisit the idea of a base type for interest rate swaps (#662). I agree that it seems janky to have two fields for the different types of swaps as in the original implementation, but it seems just as janky to have two classes of swaption which ultimately differ only in the underlying index!

Mar 14 '23 18:03 tomwhoiscontrary

Hmm. Yes, we probably need to look at those as well. Do you have additional engines in QuantExt too?

yes - also updates for CMS pricers (not sure at the moment if they are affected too)

Mar 15 '23 13:03 pcaspers

@pcaspers: I'm not sure I understood all of your answers concerning what has been adopted in ORE correctly. E.g. which of the engines in ql/pricingengines/swaption will currently work with the approach discussed here?

You have the BlackSwaptionEngine adjusted. But what about the others? I don't see adjustment for the FdHullWhiteSwaptionEngine, the TreeSwaptionEngine (incl. the DiscretizedSwaption).

I would expect changes for them as well to handle e.g. a Bermudan swaption which is between two call dates. The current running OIS coupon cannot be priced correctly with their current implementation, can they?!

Or have I overlooked something?

Mar 15 '23 15:03 ralfkonrad

Another road here would be to revisit the idea of a base type for interest rate swaps (#662). I agree that it seems janky to have two fields for the different types of swaps as in the original implementation, but it seems just as janky to have two classes of swaption which ultimately differ only in the underlying index!

Yes, I'm liking this solution better than any of the alternatives

Mar 15 '23 16:03 lballabio

@pcaspers: I'm not sure I understood all of your answers concerning what has been adopted in ORE correctly. E.g. which of the engines in ql/pricingengines/swaption will currently work with the approach discussed here?

You have the BlackSwaptionEngine adjusted. But what about the others? I don't see adjustment for the FdHullWhiteSwaptionEngine, the TreeSwaptionEngine (incl. the DiscretizedSwaption).

I would expect changes for them as well to handle e.g. a Bermudan swaption which is between two call dates. The current running OIS coupon cannot be priced correctly with their current implementation, can they?!

Or have I overlooked something?

No you haven't overlooked anything. We don't use FdHullWhiteSwaptionEngine or TreeSwaptionEngine in ORE, therefore I didn't adjust those. But we have other engines (not part of QuantLib) that handle Swaptions on OIS Swaps.

Mar 15 '23 16:03 pcaspers

This PR was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks.

May 25 '23 01:05 github-actions[bot]

@pcaspers: I'm not sure I understood all of your answers concerning what has been adopted in ORE correctly. E.g. which of the engines in ql/pricingengines/swaption will currently work with the approach discussed here? You have the BlackSwaptionEngine adjusted. But what about the others? I don't see adjustment for the FdHullWhiteSwaptionEngine, the TreeSwaptionEngine (incl. the DiscretizedSwaption). I would expect changes for them as well to handle e.g. a Bermudan swaption which is between two call dates. The current running OIS coupon cannot be priced correctly with their current implementation, can they?! Or have I overlooked something?

No you haven't overlooked anything. We don't use FdHullWhiteSwaptionEngine or TreeSwaptionEngine in ORE, therefore I didn't adjust those. But we have other engines (not part of QuantLib) that handle Swaptions on OIS Swaps.

@pcaspers: I'm having another look at this.

So if I get this correctly: (a) the part of this PR that modifies the swaption itself and the Black engine can be used to price European swaptions given a vol surface built from quoted volatilities; (b) but the part that modifies the swaption helpers can't be used until we have an engine that works with OIS swaptions (either the existing JamshidianSwaptionEngine, TreeSwaptionEngine etc or some new one from ORE) and thus can be used during calibration.

Is that right?

Aug 03 '23 09:08 lballabio

@lballabio yes that sounds right.

Aug 03 '23 10:08 pcaspers

@pcaspers this can probably be simplified using #1789 which is now merged—do you want to try it in ORE first?

Sep 19 '23 09:09 lballabio

@lballabio that would be nice. However since we have a release end of September I am going to look into this in October or possibly even later. I.e. I don't want to hold things up!

Sep 19 '23 12:09 pcaspers

Sure!

Sep 19 '23 13:09 lballabio

Just bumping.

Feb 01 '24 10:02 lballabio

So in ORE we are moving in a slightly different direction now: In general we need to support swaptions with European, Bermudan, American exercise right on overnight, Ibor, BMA/SIFMA, Subperiod coupons. For Bermudan / American swaptions we are using the MultiLegOption instrument

https://github.com/OpenSourceRisk/Engine/blob/master/QuantExt/qle/instruments/multilegoption.hpp

for a while now already, in conjunction with numerical integration / finite-difference and MC engines. Only the European case still used QuantLib::Swaption and NonStandardSwaption and the corresponding Black pricing engine. I am now working on adapting the Black engine to MultiLegOption and generalizing it to the remaining coupon types, so that all cases can be uniformly represented by MultiLegOption and our existing numerical engines can also be applied to the European case. SwaptionHelper is still on Swaption and the QL Black engine. We might move to the new Black engine there too.

Long story short, this development here has no high priority for us at the moment. Of course I'd be happy to migrate our approach to QuantLib, but I appreciate that this would be too disruptive (correct me if I am wrong!), so that we'll probably just keep the variants in parallel.

Feb 05 '24 19:02 pcaspers

What do you think of the current PR? This is a natural extension of #https://github.com/lballabio/QuantLib/pull/1789. I don't think it breaks anything, and it allows to price OIS european swaptions using the Black engine, which is better than the status quo.

Feb 14 '24 19:02 thrasibule

I still have to find some time to look at the new changes, but yes, I'd try to get it into next release.

Feb 15 '24 15:02 lballabio

I'm not fully convinced by the approach yet.

In SwaptionHelper we start already to distinguish between VanillaSwap and OvernightIndexedSwap. This will in the long run be spread into other parts of the library like engines as well, I guess. The idea of c++ and OOP is to get rid of these if and switch statements by using a clean class hierarchy.
Pricing engine like the TreeSwaptionEngine will probably fail or at least give wrong results if we pass an OIS swap into it. We might make them fail fast by checking for a VanillaSwap but then see remark 1 above.
The G2SwaptionEngine might except an OIS swap but there is no corresponding test case to check the result of these engines. As far as I can see only the BlackSwaptionEngine is currently tested.

I am still in favor of doing it the hard but consistent way to have a class likeOISSwaption with its own engines etc. and avoiding code duplication by using templates and/or an abstract base class like FixedVsFloatingSwaption.

Feb 21 '24 11:02 ralfkonrad

SwaptionHelper is a little awkward. Maybe it makes sense to have a separate OISSwaptionHelper class, where we can pass the averagingmethod there. Other than that I don't see why we need to create a new instrument class just because a particular engine doesn't give the right results. BlackSwaptionEngine doesn't give the right result with Bermudan exercise for instance.

Feb 22 '24 03:02 thrasibule

A separate OISSwaptionHelper class might be a good idea. Existing engines not working with OIS are not a problem but they should probably raise an exception if passed an OIS swaption. What do you think?

Mar 05 '24 13:03 lballabio

I started separating SwaptionHelper and OISSwaptionHelper but when I got halfway I realized that the switch was now isolated in a small method makeSwap. I might stop here. What do you think?

Mar 15 '24 16:03 lballabio

This definitely less invasive. I've done the whole hierarchy here: https://github.com/thrasibule/QuantLib/commit/038a30b93d31bb8873ba26406acab81b2fa6d4e4 (with almost the same makeSwap method). The only advantage is if we want to pass additional parameters to overnightindexed swaption like averaging method.

Mar 15 '24 16:03 thrasibule