Open pcaspers opened 4 years ago
You're right, both about the problem and about the fact that a fix is not trivial...
For the time being, I'd add overloads that don't take the day counter and deprecate the old versions.
And yes, let's remove (or deprecate) ZeroSpreadedTermStructure::forwardImpl(Time t)
.
Yeah I guess whether it's worth providing the day counter depends on what the standard practice is to calculate the z-spread (which I am not sure about yet). If it's required we can still introduce a special solution for the z-spread calculation without solving the general case of the spreaded term structure.
ZeroSpreadedTermStructure
andInterpolatedPiecewiseZeroSpreadedTermStructure
as wellBondFunctions::zSpread()
(which uses the former class internally) suggest that the spread can be given w.r.t. a day count convention specified by aDayCounter
argument. However looking at the implementation this argument is ignored and the day count convention of the benchmark rate curve is alway used.I am not sure if there is a valid use case for having different day counters for the benchmark curve and the spreads, but I think we should either add a clear documentation or (better) remove the argument or (a bit ugly) throw if the day counter in the argument is not consistent with the curve day counter or (at best?) fix the issue.
A fix does not seem to be trivial though. At the moment I'd think we need to add date based virtual
discountImpl(const Date&)
andzeroYieldImpl(const Date&)
methods to theYieldTermStructure
resp.ZeroYieldStructure
interfaces and possibly throw (?) in the time based method implementations in the spreaded term structure classes mentioned above.Any opinions how to move forward?
By the way I noticed that
ZeroSpreadedTermStructure::forwardImpl(Time t)
seems to return a forward rate with wrong conventions, but luckily the method is protected and does not seem to implement any interface :-) ... so that can probably just removed.