MakeVanillaSwap / MakeOIS withSettlementDays clears an explicit effective date

[tomwhoiscontrary]

MakeVanillaSwap and MakeOIS both allow customisation of the swap's settlement days (via withSettlementDays) and effective date (via withEffectiveDate). The settlement days is used to compute the spot date from the reference data, as part of working out the start date in the absence of an explicit effective date (start date and effective date are synonyms, right?).

However, setting the settlement days has the (to me, quite surprising) additional effect of clearing an explicit effective date. So, if MakeVanillaSwap is used to customise the effective date and also the settlement days, the resulting swap will have a start date at spot, not at the specified effective date.

In particular, that means that these two swaps have different dates (see attached code for a complete example):

  shared_ptr<VanillaSwap> swapSettlementDaysThenEffectiveDate =
      MakeVanillaSwap(tenor, index).withSettlementDays(2).withEffectiveDate(effectiveDate);

  shared_ptr<VanillaSwap> swapEffectiveDateThenSettlementDays =
      MakeVanillaSwap(tenor, index).withEffectiveDate(effectiveDate).withSettlementDays(2);

with_settlement_days_example.cpp.txt

This strikes me as highly dubious!

Is this intentional? What is the rationale for this?

I appreciate that it may seem nonsensical to set both the effective date and the settlement days, given that they are mutually exclusive ways of setting the start date. But this happens in my code because we have some common setup code used to prepare swaps of various kinds; it always sets the settlement days, and optionally sets the effective date. As it happens, it set the effective date first, so it was getting cleared.

EDIT: The previous paragraph is slightly wrong. I was uselessly setting the settlement days because i have a custom rate helper, which is mostly copied from SwapRateHelper (great artists steal!), and which takes the settlement days as a parameter. I see that since i took that copy, you made commit 0ec4781 which changes the way settlement days are handled, and would avoid this bug. I should update my code!

If you want to enforce mutual exclusiveness of these two ways of setting the start date, can i suggest doing it by means of assertions, rather than one silently dominating the other? Remove the line which clears the effective date, and instead, in the build method, do something like:

QL_ASSERT(effectiveDate_ == Date() || (settlementDays_ == Null<Natural>() && forwardStart_ == 0*Days), "make your mind up!");

I'm happy to write a PR if you think this is a good idea.

if not that, could this behaviour at least be documented in a comment on withSettlementDays?

[lballabio]

Hmm — I would have thought that passing both already raised an exception. The fact that there's an explicit override in the code makes me wonder if this is a Chesterton's fence and there's a use case I'm missing. But it probably makes sense to raise.

[tomwhoiscontrary]

There is a perhaps related story here:

    MakeVanillaSwap&
    MakeVanillaSwap::withTerminationDate(const Date& terminationDate) {
        terminationDate_ = terminationDate;
        swapTenor_ = Period();
        return *this;
    }

A MakeVanillaSwap requires a tenor to be constructed, but if you set an explicit termination date, even if the value of it is Date(), the tenor gets clobbered. And there is no way to re-add a tenor once it is clobbered.

For most normal use, this is not a big deal, but (a) it makes it harder to write generic tools on top of MakeVanillaSwap, and (b) it means it's impossible to detect misuse, where a user has set both properties to non-blank values.

[github-actions]

This issue 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.