Skip to content

Conversation

@paolodelia99
Copy link
Contributor

@paolodelia99 paolodelia99 commented Aug 23, 2025

This PR tries to be an alignment for the difference of the overnight coupons and ON coupons pricers in QuantLib and ORE.

Changes to note for ORE (@pcaspers ):

  • the OvernightIndexedCouponPricer code is in the overnightindexedcouponpricer.hpp file not in the overnightindexedcoupon.hpp file anymore.
  • rateCutoff in ORE is lockoutDays in QuantLib
  • fixingDays in ORE is lookbackDays in QuantLib
  • there is no AverageONIndexedCoupon in QuantLib the averaging method is passed as a argument in the OvernightIndexedCoupon and it is an enum type:
 struct RateAveraging {
        enum Type {
            Simple, 
            Compound 
        };
    };
  • The QL OvernightIndexedCouponPricers pricing logic haven't been changes since I haven't noticed substantial differences (apart from the rateCutoff and fixingDays naming conventions). The only thing that I'm unsure of is the following line. I haven't seen such thing in QuantLib.

Changes to note in QL (@lballabio ):

  • Added CappedFlooredOvernightIndexedCoupon class under overnigthindexcoupon.hpp (imported from ORE)
  • Added BlackOvernightIndexedCouponPricer and BlackAverageONIndexedCouponPricer for pricing capped / floored compounded ON coupons (imported from ORE)
  • Added a bunch of other methods in the OvernightLeg class (includeSpread, withCaps, withFloors, withNakedOption, ...) imported from ORE
  • Added includeSpread, rateComputationStartDate, rateComputationEndDate arguments to the OvernightIndexedCoupon constructor (missing args from ORE)

@coveralls
Copy link

coveralls commented Aug 23, 2025

Coverage Status

coverage: 74.092% (+0.2%) from 73.873%
when pulling b8a116f on paolodelia99:feature/ql-ore-coupons-alignment
into 7a6a9a2 on lballabio:master.

@paolodelia99
Copy link
Contributor Author

@lballabio I actually need help with the test testOvernightLegWithCapsAndFloors in overnightindexedcoupon.hpp. Don't know why I'm getting different results on different builds, at first I thought that I was due to the usingAtParCoupons optional, but apparently I was wrong. Can you please take a look at it when you have time?

@lballabio
Copy link
Owner

Not a lot of time now unfortunately, but I would try checking that we're not accessing some vector out of its bounds and getting garbage numbers.

@lballabio
Copy link
Owner

I pushed a fix for the failing tests—a couple of bools were left uninitialized.

I still have to do a proper review...

@paolodelia99
Copy link
Contributor Author

Thank you @lballabio! I didn't have much time to debug the tests lately

@paolodelia99 paolodelia99 marked this pull request as ready for review September 6, 2025 10:30
@lballabio
Copy link
Owner

Apologies for the long delay. One thing: currently the pricers are defined this way:

classDiagram

class FloatingRateCouponPricer
class CompoundingOvernightIndexedCouponPricer
class ArithmeticAveragedOvernightIndexedCouponPricer
class CappedFlooredOvernightIndexedCouponPricer
class BlackOvernightIndexedCouponPricer
class BlackAverageONIndexedCouponPricer

FloatingRateCouponPricer <|-- CompoundingOvernightIndexedCouponPricer
FloatingRateCouponPricer <|-- ArithmeticAveragedOvernightIndexedCouponPricer
FloatingRateCouponPricer <|-- CappedFlooredOvernightIndexedCouponPricer
CappedFlooredOvernightIndexedCouponPricer <|-- BlackOvernightIndexedCouponPricer
CappedFlooredOvernightIndexedCouponPricer <|-- BlackAverageONIndexedCouponPricer
Loading

Instead, I would try to turn it into

classDiagram

class FloatingRateCouponPricer
class CompoundingOvernightIndexedCouponPricer
class ArithmeticAveragedOvernightIndexedCouponPricer
class BlackOvernightIndexedCouponPricer
class BlackAverageONIndexedCouponPricer

FloatingRateCouponPricer <|-- CompoundingOvernightIndexedCouponPricer
FloatingRateCouponPricer <|-- ArithmeticAveragedOvernightIndexedCouponPricer
CompoundingOvernightIndexedCouponPricer <|-- BlackOvernightIndexedCouponPricer
ArithmeticAveragedOvernightIndexedCouponPricer <|-- BlackAverageONIndexedCouponPricer
Loading

or maybe, if we need to share something between compounded and averaged, something like

classDiagram

class FloatingRateCouponPricer
class OvernightIndexedCouponPricer
class CompoundingOvernightIndexedCouponPricer
class ArithmeticAveragedOvernightIndexedCouponPricer
class BlackOvernightIndexedCouponPricer
class BlackAverageONIndexedCouponPricer

FloatingRateCouponPricer <|-- OvernightIndexedCouponPricer
OvernightIndexedCouponPricer <|-- CompoundingOvernightIndexedCouponPricer
OvernightIndexedCouponPricer <|-- ArithmeticAveragedOvernightIndexedCouponPricer
CompoundingOvernightIndexedCouponPricer <|-- BlackOvernightIndexedCouponPricer
ArithmeticAveragedOvernightIndexedCouponPricer <|-- BlackAverageONIndexedCouponPricer
Loading

What do you think?

@paolodelia99
Copy link
Contributor Author

2nd and 3rd case make more sense to me. I would opt for the 3rd case. One question: is OvernightIndexedCouponPricer gonna be a virtual class that is just defining the interface for the child classes?

@lballabio
Copy link
Owner

The interface is already declared in FloatingRateCouponPricer. The base class might perhaps store the volatility term structure, but otherwise there's not a lot in common, which is why the existing pricers don't have a common base.

@paolodelia99
Copy link
Contributor Author

Got it. So the OvernightIndexedCoupon pricer class should do the same thing as the IborCouponPricer class, where the volatility term structure is set by the setCapletVolatility method. In this way, we are going to have a consistent interface among those "base classes".

@paolodelia99
Copy link
Contributor Author

One problem that I have noticed @lballabio: If BlackOvernightIndexedCouponPricer is going to become a child of CompoundingOvernightIndexedCouponPricer there is going to be a clash in the type of coupon_ attribute, since the the former is a CappedFlooredOvernightIndexedCoupon and in the latter is OvernightIndexedCoupon (They are both child of FloatingRateCoupon). How would you suggest me to tackle this problem?

@lballabio
Copy link
Owner

I think you can copy the approach in CappedFlooredIborCoupon. It inherits from CappedFlooredCoupon, not FloatingRateCoupon directly, and the setPricer method in the base class is overridden so that it sets the passed pricer to its underlying instead of the capped coupon itself. It should work in this case as well. Let me know if you need more details.

Rate BlackAverageONIndexedCouponPricer::swapletRate() const { return swapletRate_; }

Rate BlackAverageONIndexedCouponPricer::capletRate(Rate effectiveCap) const {
return coupon_->localCapFloor() ? optionletRateLocal(Option::Call, effectiveCap)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that the CappedFlooredOvernightIndexedCoupon has the localCapFloor_ attribute which I think is more a property of the coupon than a property of the pricer.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see. The Black Ibor pricer doesn't have this problem because it doesn't need other data from the coupon besides the effective cap which is passed through the interface. Let me think a bit about it. And of course you're welcome do to some thinking of your own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that defining an overload of capletRate(Rate) (and similarly floorletRate(Rate)) with the signature capletRate(Rate effectiveCap, bool localCapFloor) might be a good approach. After all, these functions are only ever called by the coupon. The implementation provided by the FloatingRateCouponPricer interface could simply call this overload, passing false for the localCapFloor parameter.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

};

//! capped floored overnight indexed coupon pricer base interface
class CappedFlooredOvernightIndexedCouponPricer : virtual public OvernightIndexedCouponPricer {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be folded into OvernightIndexedCouponPricer so you can avoid the double inheritance in the Black pricers. With methods like capletPrice, floorletPrice and setCapletVolatility, OvernightIndexedCouponPricer is already supposed to be used for capped coupons too.

Copy link
Contributor Author

@paolodelia99 paolodelia99 Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double inheritance is avoided because I've used virtual inheritance, but yeah at this point it makes more sense to put everything under the OvernightIndexedCouponPricer at this point since I've already pass the optionletVolatilityTermStructure in the class constructor.

Copy link
Owner

@lballabio lballabio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, another round of comments — sorry for the piecewise review, but it's a massive PR...

Comment on lines +74 to +75
const Date& rateComputationStartDate = Date(),
const Date& rateComputationEndDate = Date());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably more for @pcaspers — these two don't seem to be used anywhere in the calculation. Should they be there at all?

highly experimental and ad-hoc. As soon as a market best practice has evolved, the pricer
should be revised. */
class BlackOvernightIndexedCouponPricer : public CompoundingOvernightIndexedCouponPricer {
using CompoundingOvernightIndexedCouponPricer::CompoundingOvernightIndexedCouponPricer;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're importing the base-class constructor but you're also declaring in this class a constructor with the same signature. (Maybe it's the cause of the error in the Appveyor build?) You only need one.

highly experimental and ad-hoc. As soon as a market best practice has evolved, the pricer
should be revised. */
class BlackAverageONIndexedCouponPricer : public ArithmeticAveragedOvernightIndexedCouponPricer {
using ArithmeticAveragedOvernightIndexedCouponPricer::ArithmeticAveragedOvernightIndexedCouponPricer;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the signature is different, but do you really want to import the base-class constructor?

Comment on lines 358 to 361
const ext::shared_ptr<BlackOvernightIndexedCouponPricer> overnightCouponPricer =
ext::dynamic_pointer_cast<BlackOvernightIndexedCouponPricer>(pricer_);
QL_REQUIRE(overnightCouponPricer,
"pricer not compatible with capped-floored overnight indexed coupon");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way you're making it impossible to define and use another capped/floored pricer which is not Black. Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I only check whether the pricer is a child of OvernightIndexedCouponPricer then?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so.

return spread();

if (averagingMethod_ == RateAveraging::Simple)
QL_FAIL("Average OIS Coupon does not have an effectiveSpread"); // FIXME: better error message
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For @pcaspers — does not have yet, or it can't be defined, or in the simple case is the same as the spread?

Comment on lines +256 to +263
if (!coupon_->includeSpread()) {
finalRate += coupon_->spread();
effectiveSpread = coupon_->spread();
effectiveIndexFixing = finalRate;
} else {
effectiveSpread = finalRate - (compoundFactorWithoutSpread - 1.0) / tau;
effectiveIndexFixing = finalRate - effectiveSpread;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm puzzled here — in the if the effective index fixing equals the final rate and therefore inlcudes the spread, but in the else it seems to exclude it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should ask it to @pcaspers, it took this logic from ORE. But I guess in the first case, where the spread is not included in the compounding logic, the effectiveIndexFixing is set to finalRate because the observable/effective fixing used downstream is the final coupon rate after the additive spread.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants