Stricter behaviour of SwapLeg::getValue()

[NiklasRodi]

What is this PR for?

• Do not try to calculate forwards from a discountCurve if forwardCurveName corresponds to a discountCurve as this wrapping should be done outside this class if intended, throw exception instead
• do not allow empty SwapLeg periods any more, throw exception instead

What type of PR is it?

Feature

[cfries]

I am not so sure with this one. The additional code was to allow for a true single curve swap, where the float leg evaluates to 1 side the calculation of forward (f(t1)(t2-t1) = (df(t1)-df(t2)) / df(t2) exactly cancels with a discounting df(t2). This is almost impossible with a ForwardCurveFromDiscountCurve, since we have to ensure that the schedule of the float leg is used. Options I see:

• do not allow for the idealised single curve SwapLeg (this PR).
• make single curve an explicit option in the constructor instead of a "fall back" (safer).
• implement a specific single curve SwapLeg which only takes one discount curve (this PR, maybe my favourite)

[cfries]

I would merge the removal of the "fall back calculation" of the forward from a discount curve, since this should be handled by ForwardCurveFromDiscountCurve.

However: could you maybe revert the change w.r.t. empty periods? Why should it not be allowed to specify periodLength = 0. Maybe some tries to "switch off" periods a-posteriory by providing a specialised Daycount Convention Implementation. If periodStart=periodEnd is considered a mis-specification this should be handled in a GUI not deep down in the algorithm. The algorithm is just fine with zero (or even negative) period length.

[NiklasRodi]

My idea was to impose a sanity check of the schedule generation. Would you be ok with imposing such a restriction there? I don't expect it to ever throw an exception but you never know.

However, I'm not sure I understand your use case. A special dayCountConvention should be implemented by modifying the periodLength of this period - not periodStart and periodEnd. If you have a normal 3M schedule and now switch off one period by modifying periodStart and periodEnd you will end up with one 6M period or two periods >3M. This does not really make sense to me.

p.s.: At first I thought that setting periodLength=0 will manually "switch off" the respective period. However, this does not work for notional exchanging swaps.

[cfries]

Your code is checking on periodLength - that is on DCC(periodStart, periodEnd). Your code is not checking on peirodEnd==periodStart (however: schedule generator is already doing this).

if(periodLength == 0)
    throw new IllegalArgumentException(periodIndex + "th period of swapLeg is empty which is not allowed.");

I would not impose any restriction at all here. Why at all? (In addition: getValue ist maybe the wrong place, since it is very late and affects performance. If at all, such a check should be part of the constructor, but again: why impose a restriction if valuation of zero period length corresponds the correct "limit case" (even in mathematical terms).

With periodLength = 0 (via the day count convention) the user may switch off the coupon. This may be nice for analysis (e.g. get only notional flows <=> initialise swap with dcf === zero). With periodStart = periodEnd the user can switch off notional exchange too.

[cfries]

BTW: From a design perspective this is the wrong place. Schedule should provide valid schedules. Swap should not check for the validity (except if there is a deep mathematical relation to swap, e.g. the index being 3M on a 6M schedule requiring a convexity adjustment). W.r.t. the convexity adjustment I would just note in the Java Doc that such a check is not performed and the adjustment is considered zero (zero-volatility assumption).

[NiklasRodi]

Ah my mistake. I associated periodLength(i)=0 with periodStart(i)=periodEnd(i) which is not necessarily true. I have to say that the name "periodLength" is indeed a bit confusing here, "dayCountFraction" would be more appropriate. I will revert the this change

[cfries]

Thanks for this discussion. From the discussion I would say that the if(periodLength == 0) continue should be removed!

[cfries]

(thx. will merge if unit test passes).

[NiklasRodi]

Why the additional check periodLength!=0? value is incremented by forwardperiodLengthdiscountFactor which is 0 if periodLength==0. Removing the additional check will only cause the discountFactor and the forward to be evaluated. Bus this shouldn't cause any problems imO.

[cfries]

Yes... I thought about just dropping the if(...) alltogther. (There is just a minor gain: user could set periodLength to zero and forardCurve could be null).

[NiklasRodi]

But the forward is only calculated when forwardCurve != null and there is also a check that the discountCurve != null at this stage. So I still don't see the benefit and would remove the periodLength!=0 condition.

[cfries]

Convinced. The case periodLength==0 is very rare and the check affects performance (I would assume that JVM removes it (due to branch prediction), but the code looks cleaner ... thx.