Design flaw in how polymorphism and generics work

[igitur]

I'm not sure whether this is a major problem, but I struggled for a long time and couldn't solve it. I'm trying to implement the default probability curves. The design pattern is similar in how the yield term structures work.

To illustrate the problem, look at the testLinearDiscountConsistency test. It uses a PiecewiseYieldCurve to build a curve and interpolates the Discount curve using Linear interpolator. When the interpolation occurs, this piece of the codebase is executed:

In QuantLib:

 DiscountFactor PiecewiseYieldCurve<C,I,B>::discountImpl(Time t) const {
     calculate();
     return base_curve::discountImpl(t);
 }

The discountImpl that is executed is then InterpolatedDiscountCurve<T>::discountImpl :

DiscountFactor InterpolatedDiscountCurve<T>::discountImpl(Time t) const {
     if (t <= this->times_.back())
        return this->interpolation_(t, true);

    // flat fwd extrapolation
    Time tMax = this->times_.back();
    DiscountFactor dMax = this->data_.back();
    Rate instFwdMax = - this->interpolation_.derivative(tMax) / dMax;
    return dMax * std::exp(- instFwdMax * (t-tMax));
}

What happens in QLNet is this. In PiecewiseYieldCurve:

protected override double discountImpl(double t) {
   calculate();
   return traits_.discountImpl(interpolation_, t);
}

But _traits is defined as the Discount class, not InterpolatedDiscountCurve as in QuantLib and the code that is then executed is:

public double discountImpl( Interpolation i, double t ) { return i.value( t, true ); }

This is because of the difference in how the generics and polymorphism work. We are "lucky" in the sense that InterpolatedDiscountCurve.discountImpl and Discount.discountImpl do almost the same thing and that is why the tests pass.

When I started implementing the Default Curves, I noticed the same problem, except we aren't so lucky anymore. Refer to the testLinearDensityConsistency, which is very similar to the discount curve test I discussed above. The test wants to calculate a swap fair value and in doing so, needs the survival probability. In QuantLib: PiecewiseDefaultCurve::survivalProbabilityImpl would call InterpolatedDefaultDensityCurve<T>::survivalProbabilityImpl. But in QLNet, PiecewiseDefaultCurve.survivalProbabilityImpl would call DefaultDensity.survivalProbabilityImpl:

public double survivalProbabilityImpl(Interpolation i, double t) { throw new NotSupportedException(); }

And then the test fails. There must be a way to make the call to InterpolatedDefaultDensityCurve instead of DefaultDensity, but I can't figure how how. C++ has typenames and typedefs which help here, but that's not possible in C#. Maybe there is a solution using reflection?

[igitur]

I still have to commit my new branch with the default curves, but even without it, the problem exists in the yield curves too. We're just lucky the tests pass.

[amaggiulli]

The real problem for us is that PiecewiseYieldCurve in c++ is derived from a template parameter typedef :

template <class Traits, class Interpolator, template <class> class Bootstrap = IterativeBootstrap>
    class PiecewiseYieldCurve : public Traits::template curve<Interpolator>::type

so InterpolatedDiscountCurve<T> is the base class of PiecewiseYieldCurve<Discount,LogLinear,IterativeBootstrap> and you can call :

base_curve::discountImpl(t);

This is impossible to do in c#, so during the years we tried to make a configuration that works in most cases . Several people worked on this but still we don't have a clean solution.
In my opinion a complete refactoring with interfaces is needed.

[igitur]

Well, yes, that's what I was also trying to say. ;-)

I'll start reducing the problem so that it's easier to look for alternative solutions.

[igitur]

Hang on, I'm still working on this...

[tournierjc]

Hello,

I will try to help a bit but you may have notice I'm not such a good coder.

In QLNet the design of the different Traits does not seems to be as in Quantlib and honestly I do not understand why. For example this part isn't "translated" to C#:

template <class Interpolator>
struct curve {
    typedef InterpolatedDiscountCurve<Interpolator> type
};

I know @igitur does not really like this but it seems that the following sample can replace easily this code:

• Add this to the ITraits interface

//reflection
void setInterpolator(IInterpolationFactory interpolator);
Type getCurveType();

• Add this to all classes inheriting from ITraits interface

// reflection method for curve type
Type curve_type;
public void setInterpolator( IInterpolationFactory interpolator )
 {
      curve_type = typeof(InterpolatedDiscountCurve<>).MakeGenericType(interpolator.GetType());
}
public Type getCurveType()
{
      return curve_type;
}

• Add this for all classes inheriting from Curve interface

public void setInterpolator(IInterpolationFactory interpolator) { throw new NotSupportedException(); }
public Type getCurveType() { throw new NotSupportedException(); }

Then it's becoming quite easy to instatiate within PieceYieldCurve the base_curve object:

typedef typename Traits::template curve<Interpolator>::type base_curve;

will become

_traits_ = FastActivator<Traits>.Create();
_traits_.setInterpolator(i);
base_curve_ = Activator.CreateInstance(_traits_.getCurveType(), referenceDate, dayCounter, jumps, jumpDates, i) as YieldTermStructure;

With previously declared protected YieldTermStructure base_curve_;.

Then we need to update the discountImpl:

public override double discountImpl(double t) {
     calculate();
     return base_curve_.discountImpl(t);
}

Until here I get it working, but now I need to update the base_curve with bootstrap helpers when calculate() method is called. If you have any suggestion ?

[igitur]

Well, yes, I don't like using Reflection for a few reasons:

• It's slow, but we might get around it by using a library like FastMember
• You lose compile time type checking, but with a gazillion unit tests, this should be OK.
• You lose IDE features, e.g. refactoring support, etc.

But I have admit that I've also thought that Reflection is going to be our only solution here. I don't think C#'s generics are advanced enough yet. Higher kinded generics might be a solution if it is implemented in C# one day. I'm not sure.

[tournierjc]

How much slower is it ?

Personally I use the PiecewiseCurves only for bootstrapping once and then I'm using directly the InterpolatedCurves with calculated ZeroRates. So the growth in calculation time will barely impact me.

Your issue is almost a year old, I think it's time to correct this (which is quite alarming as the behaviour is totally different from official Quantlib).

Starting with reflection now should help once the feature is available in C# and should not be too troublesome to migrate.

@amaggiulli seems to disaprove use of external library, so I think we know what is left to do.

[igitur]

Let me just complete that reduced example first and get some feedback from StackOverflow. There might be a few gurus there would could give advice. We should at least be 100% sure that there is no way that generics can work.

If generics are ruled out, then, besides reflection, we should also investigate lambda expressions. I don't know them well, but I have a hunch that might be a solution. And they're much faster than reflection.

To answer your question, Reflection is a lot slower, and here is a post that explains why.

[amaggiulli]

The right way to solve this is with Interfaces , reflection is not needed ( and it is to avoid if possible ), we don't need a guru , just some time to dedicate to this development and , like all the open source projects , this can't be planned with certainty.

[tournierjc]

Well, I will probably leave that to you as I have no idea how to do it with interfaces. Regarding the reflection, my proposal was just using it to instatiate the object, all methods are then called by the usual way. I don't think it is impacting the time calculation like in the exemple given by @igitur. If I can be of any help, tell me I have a bit of the needed time. Le 5 mai 2017 14:14, "Andrea Maggiulli" <[email protected]> a écrit :

…

The right way to solve this is with Interfaces , reflection is not needed ( and it is to avoid if possible ), we don't need a guru , just some time to dedicate to this development and , like all the open source projects , this can't be planned with certainty. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#89 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AXZfhJJdo-xEad06t7TJt--fIHA-CENyks5r2xK4gaJpZM4Ip5mn> .

[amaggiulli]

work at this issue in branch https://github.com/amaggiulli/QLNet/tree/generics

[tournierjc]

Hello,

Did you had a look at this branch : https://github.com/tournierjc/QLNet/tree/ITraitsRefactoring?

[igitur]

@tournierjc With #196 merged, you can rebase and your unit tests should pass now.

[igitur]

@tournierjc Can you log a PR for it in the meantime?

[andeen]

Hi,

I'm also having a hard time implementing the PiecewiseDefaultCurve. Any updates on solving this issue using interfaces?

[tournierjc]

Hello @andeen ,

I have implemented it in my repos (defaultcurve branch): https://github.com/tournierjc/QLNet/tree/defaultcurves

I'm using it on a daily basis, it's working great.

[andeen]

That's just perfect, tournierjc. Thanks!

What's the status on the pull request?

[tournierjc]

@andeen the previous pull request has been closed without merging. I will make another one.