Long existing bug causing error in first period interest/cashflow calculation for FixedRateBond

[DeimosXing]

Hi QL developers, I found a bug in the latest version of QL which caused inaccurate cashflow and accrued interest calculation for the first coupon period in a FixedRateBond.

Consider this US treasury note with the following properties:
https://www.investing.com/rates-bonds/usgovt-1.875-30-sep-2022

ISIN: US9128282W90
Issue Date: 2017-10-02
Interest Accrues: 2017-09-30
First Coupon Date: 2018-03-31
Maturity Date: 2022-09-30
Day Counter: ISMA Actual/Actual
Business Day Convention: Unadjusted
End of Month: true
Tenor: Semiannual

Construct it and calculate its cashflow in QL 1.9 and 1.32:
Note that QL 1.9 cashflow outputs matches a best-in-class data vendor.

import QuantLib as ql
calendar = ql.UnitedStates(ql.UnitedStates.GovernmentBond)
tenor = ql.Period(2)
payment_schedule = ql.Schedule(ql.Date(30,9,2017), ql.Date(30,9,2022), tenor, calendar, 4, 4, ql.DateGeneration.Backward, True, ql.Date(31,3,2018), ql.Date(31,3,2022))
accrual_daycount = ql.ActualActual(ql.ActualActual.Bond)
bond = ql.FixedRateBond(1, 100, payment_schedule, [0.01875], accrual_daycount)
print([(i.date(), i.amount()) for i in bond.cashflows()])
 
Expected output(QL 1.9, Vendor):
[(Date(2,4,2018), 0.9374999999999911),
 (Date(1,10,2018), 0.9374999999999911),
 (Date(1,4,2019), 0.9374999999999911),
 (Date(30,9,2019), 0.9374999999999911),
 (Date(31,3,2020), 0.9374999999999911),
 (Date(30,9,2020), 0.9374999999999911),
 (Date(31,3,2021), 0.9374999999999911),
 (Date(30,9,2021), 0.9374999999999911),
 (Date(31,3,2022), 0.9374999999999911),
 (Date(30,9,2022), 0.9374999999999911),
 (Date(30,9,2022), 100.0)]
 
Actual output (QL 1.10+):
[(Date(2,4,2018), 0.9323770491803218), # First cashflow is wrong
 (Date(1,10,2018), 0.9374999999999911),
 (Date(1,4,2019), 0.9374999999999911),
 (Date(30,9,2019), 0.9374999999999911),
 (Date(31,3,2020), 0.9374999999999911),
 (Date(30,9,2020), 0.9374999999999911),
 (Date(31,3,2021), 0.9374999999999911),
 (Date(30,9,2021), 0.9374999999999911),
 (Date(31,3,2022), 0.9374999999999911),
 (Date(30,9,2022), 0.9374999999999911),
 (Date(30,9,2022), 100.0)]

Upon investigation, this turns out to be caused by the ref date of the first coupon period being moved to the last business day of the month.
https://github.com/lballabio/QuantLib/blob/master/ql/cashflows/fixedratecoupon.cpp#L198-L203

       // This should be 2018-09-30, but was moved to 2018-09-29 because that's the last business day
        Date ref = schedule_.hasTenor() &&
            schedule_.hasIsRegular() && !schedule_.isRegular(1) ?
            schedule_.calendar().advance(end,
                                         -schedule_.tenor(),
                                         schedule_.businessDayConvention(),
                                         schedule_.endOfMonth())
            : start;
        InterestRate r(rate.rate(),
                       firstPeriodDC_.empty() ? rate.dayCounter()
                       : firstPeriodDC_,
                       rate.compounding(), rate.frequency());
        leg.push_back(ext::shared_ptr<CashFlow>(new
            FixedRateCoupon(paymentDate, nominal, r,
                            start, end, ref, end, exCouponDate)));
...
        // cashflow is calculated as coupon_rate * accrual_daycount.yearFraction(start, end, ref, end)
        QL 1.10+:
        accrual_daycount.yearFraction(start, end, ref, end)
        0.4972677595628415 // when ref is 2018-09-29, this fraction is off
        QL 1.9:
        accrual_daycount.yearFraction(start, end, ref, end)
        0.5 // when ref is 2018-09-30, this gives the correct answer

Searching in history, this bug/feature was first introduced in https://github.com/lballabio/QuantLib/pull/214/files where it moved ref date of the first coupon to the last business day of the month. Here's the related PR #210 which introduced this bug.

Here's my proposed way to fix this:

https://github.com/lballabio/QuantLib/blob/master/ql/time/calendar.cpp#L162
In Calendar::advance(), the return date will be moved to end of business day if endOfMonth && isEndOfMonth(d) is True. However, isEndOfMonth(d) is True when d is or after the end of business day, which introduces a discrepancy between d and the return date. I suggest implementing an isEndOfBusinessDay() and use that to replace the isEndOfMonth(d) in Calendar::advance()

Since I'm not a bond/QuantLib expert, I'm not sure the possible side effects of this fix and would love to hear from ppl that's more familiar with this lib and make a better fix. Thanks!

[boring-cyborg]

Thanks for posting! It might take a while before we look at your issue, so don't worry if there seems to be no feedback. We'll get to it.

[lballabio]

I see, thanks. I think that to be consistent with what happens during the generation of the coupon dates, Calendar::advance should check if the convention is Unadjusted and in that case it should return the last day of the month, not the last business day. Does this make sense to you?

[DeimosXing]

Thanks for your prompt reply @lballabio !

Your proposal sounds good to me maybe except for one case:
Say the original date d is one day after the last business day of month, but not the last calendar day of month. For example, 29th is the last business day, d is 30th, and the last calendar day is 31th.
Then returning the last day of month (31th) may still be wrong?

I'm not 100% sure if this is a valid example and appreciate more input from expert like you. Generally I agree that we should fill the gap between isEndOfMonth and endOfMonth for Unadjusted convention in
https://github.com/lballabio/QuantLib/blob/master/ql/time/calendar.hpp#L243-L249

[DeimosXing]

I also just noticed a long-existing thread about the same issue #405. This feels like a great opportunity to settle on a solution for this sought-after fix.

[lballabio]

Your proposal sounds good to me maybe except for one case:
Say the original date d is one day after the last business day of month, but not the last calendar day of month. For example, 29th is the last business day, d is 30th, and the last calendar day is 31th.
Then returning the last day of month (31th) may still be wrong?

I'm not sure if it would make sense to have Unadjusted and end-of-month in this case?

[DeimosXing]

@lballabio I see, yeah, agreed. I can go ahead and make a PR to fix then. Basically modify Calendar::advance to check if the convention is Unadjusted and return the last day of the month if true, as you suggested.

[lballabio]

Sure, go ahead—thanks!