add multicurve bootstrap

[pcaspers]

add global bootstrap over multiple curves

unit test to be added, this should also illustrate the process

[coveralls]

coverage: 74.35% (+0.4%) from 73.914%
when pulling 313f6c3 on pcaspers:multicurve_bootstrap
into eea8696 on lballabio:master.

[pcaspers]

The canonical example would be Euribor3m vs 6m where the two forward curves depend on each other via tenor basis swaps. We don't have tenor basis swap helpers in QuantLib though, so try to come up with another example. If someone has suggestions? Otherwise I might migrate the tenor basis helper to QuantLib.

[lballabio]

We do have tenor basis swaps — see IborIborBasisSwapRateHelper in ql/experimental/termstructures/basisswapratehelpers.hpp.

[pcaspers]

Ah fantastic :-) thank you

[eltoder]

@pcaspers I only briefly skimmed through the changes, but I have 2 general comments:

1. Naively creating circular dependencies between curves will cause cycles in shared_ptrs and in QL notification graph. Both are bad. I haven't noticed any code to deal with this.
2. It seems easier to add costFunctionsSet, costFunctionsEval and makeGuess as methods to MultiCurveBootstrapContributor instead of having 4 parallel vectors. There's only one of the these per contributor if I'm not mistaken. This will also be more efficient for single curve bootstapping if we mark GlobalBootstrap class as final.

[pcaspers]

Thanks @eltoder. I'll have a look at 2. As for 1.: 1a) Do we think that a cycle in the QL notification graph which contains at least one LazyObject is handled ok, at least if QL_THROW_IN_CYCLES is not defined because of the way LazyObject::update() is implemented? 1b) any thoughts on how to resolve the issue with shared_ptr cycles - it looks like we need to touch Handle?

[eltoder]

@pcaspers

1a) I think that this used to work but causes infinite recursion since #1566. In fact, some tests and examples used to contain (unnecessary) cycles, and this started failing in QL 1.31 (#1688).

1b) I think we don't need to touch Handle, because shared_ptr has a lot of features. I looked into this previously. My idea was to have a MultiCurve (temporary name) object that owns all curves in the cycle. MultiCurve can return Handles/shared_ptrs of two kinds to the contained curves:
a) non-owning: these use null_deleter and should only be used by ratehelpers/etc of other curves in the cycle
b) owning: these use shared_ptrs constructed with the aliasing constructor: shared_ptr(shared_from_this(), curve) -- they all share the ownership of the whole MultiCurve object, but return pointers to the individual curves from get().

If we do that, I think we can solve the problem 1a) as well by creating non-owning Handles with registerAsObserver=false.

[pcaspers]

@eltoder how would you implement a non-owning shared_ptr? I guess this is exactly what we would need to avoid memory leaks from shared_ptr cycles, but at the moment I do not see how we would implement this?

[eltoder]

@pcaspers using null_deleter. It's already used in QL for similar purposes, for example, this is a common pattern.

[pcaspers]

Doesn't that just mean that the shared_ptr won't free the underlying pointer when the reference count goes to zero - but it does not solve the problem that shared_ptr in a cycle will never have a zero reference count?

[pcaspers]

I was thinking the standard solution would be weak_ptr and a corresponding WeakHandle, but this is of course not straightforward to integrate with QuantLib.

[eltoder]

weak_ptr is not different from using null_deleter in this context -- you still need to know which pointers should be weak and change your code accordingly. Let me write a code snippet. Hopefully that will be more clear.

[pcaspers]

Sounds good, thank you.

[eltoder]

@pcaspers something like this

// this class must be used with ext::shared_ptr
class MultiCurve : public ext::enable_shared_from_this<MultiCurve> {
  public:
    void addCurve(const std::string& name, ext::shared_ptr<YieldTermStructure> curve) {
        QL_REQUIRE(curve != nullptr, "got null curve for " << name);
        auto& entry = curves_[name];
        QL_REQUIRE(entry.ptr == nullptr, "curve " << name << " was already added");
        // ideally we set up bootstrapping here as well,
        // but this needs some changes to your code
        auto contrib = dynamic_cast<MultiCurveBootstrapContributor*>(curve.get());
        QL_REQUIRE(contrib != nullptr, "curve " << name << " is not compatible with MultiCurve");
        contrib->setParentBootstrapper(&bootstrap_);
        // TODO: setup notifications so that when any curve is updated
        // we update all other curves as well.

        // this handle should be used within the cycle
        bool observer = false;
        entry.internal.linkTo(
            ext::shared_ptr<YieldTermStructure>(curve.get(), null_deleter()), observer);
        // this handle should be used outside of the cycle
        entry.external.linkTo(
            ext::shared_ptr<YieldTermStructure>(shared_from_this(), curve.get()));
        entry.ptr = std::move(curve);
    }

    // this handle should be used within the cycle
    const Handle<YieldTermStructure>& getInternalHandle(const std::string& name) {
        return curves_[name].internal;
    }

    // this handle should be used outside of the cycle
    const Handle<YieldTermStructure>& getExternalHandle(const std::string& name) {
        return curves_[name].external;
    }
  private:
    struct Entry {
        RelinkableHandle<YieldTermStructure> internal;
        RelinkableHandle<YieldTermStructure> external;
        ext::shared_ptr<YieldTermStructure> ptr;
    };
    std::unordered_map<std::string, Entry> curves_;
    MultiCurveBootstrap bootstrap_;
};

void test() {
    auto mc = ext::make_shared<MultiCurve>();

    // build euribor3m
    auto intEuribor6m = mc->getInternalHandle("euribor6m");
    // use intEuribor6m in rate helpers that need euribor6m
    ...
    mc->addCurve("euribor3m", ext::make_shared<PiecewiseYieldCurve>(...));

    // build euribor6m
    auto intEuribor3m = mc->getInternalHandle("euribor3m");
    // use intEuribor3m in rate helpers that need euribor3m
    ...
    mc->addCurve("euribor6m", ext::make_shared<PiecewiseYieldCurve>(...));

    // done building the curves
    auto euribor3m = mc->getExternalHandle("euribor3m");
    auto euribor6m = mc->getExternalHandle("euribor6m");
    // any uses external to the cycle should use these external handles
}

[pcaspers]

Thanks a lot @eltoder - I will set up a euribor3m / 6m bootstrap using this approach as a unit test on the branch, this makes it easier to discuss.

[eltoder]

@pcaspers btw, to create a cycle I think you'll always have to create an empty RelinkableHandle, use it in rate helpers of the other curve, and then link it to the curve to complete the cycle. This should work with QL's basis swap helper, but IIUC this doesn't work with the one in ORE, because it checks which handle is empty to determine which curve is being bootstrapped.

[pcaspers]

@eltoder yes, thanks, we changed the rate helpers with regards to that, see e.g. here:

https://github.com/OpenSourceRisk/Engine/blob/master/QuantExt/qle/termstructures/tenorbasisswaphelper.cpp#L75

[pcaspers]

So speaking of ORE we don't care about notification cycles, since the curves are built once and then copied to interpolated curves anyhow. But of course we do care about possible memory leaks, so I am particularly interested in this. And when migrating the bootstrap to QL we have to have correct notifications, too, of course. I'll set up that unit test as soon as I have a chance.

[eltoder]

Sounds good. Thanks a lot for working on this.

[pcaspers]

Yes that seems to work well. And now I understand how you break the shared_ptr cycles :-) Let me run a couple of additional tests on our side and then update this branch

[eltoder]

@pcaspers nice! I realized that requiring to name curves and keeping them in a hash table is not very QL-like. This is easy to change and the result seems a bit simpler and more like other QL APIs:

// this class must be used with ext::shared_ptr
class MultiCurve : public ext::enable_shared_from_this<MultiCurve> {
  public:
    // addCurve() takes an internal handle and returns an external handle.
    // Internal handle, which must be an empty RelinkableHandle, should be
    // used within the cycle. External handle should be used outside of the
    // cycle.
    Handle<YieldTermStructure> addCurve(
            RelinkableHandle<YieldTermStructure>& internalHandle,
            ext::shared_ptr<YieldTermStructure> curve) {
        QL_REQUIRE(internalHandle.empty(),
                   "internal handle must be empty; was the curve added already?");
        QL_REQUIRE(curve != nullptr, "curve must not be null");
        // ideally we set up bootstrapping here as well,
        // but this needs some changes to your code
        auto contrib = dynamic_cast<MultiCurveBootstrapContributor*>(curve.get());
        QL_REQUIRE(contrib != nullptr, "curve is not compatible with MultiCurve");
        contrib->setParentBootstrapper(&bootstrap_);
        // TODO: setup notifications so that when any curve is updated
        // we update all other curves as well.

        bool observer = false;
        internalHandle.linkTo(
            ext::shared_ptr<YieldTermStructure>(curve.get(), null_deleter()), observer);
        Handle<YieldTermStructure> externalHandle(
            ext::shared_ptr<YieldTermStructure>(shared_from_this(), curve.get()));
        curves_.push_back(std::move(curve));
        return externalHandle;
    }
  private:
    std::vector<ext::shared_ptr<YieldTermStructure>> curves_;
    MultiCurveBootstrap bootstrap_;
};

void test() {
    auto mc = ext::make_shared<MultiCurve>();

    // internal handles that should be used by rate helpers
    // of the curves in the cycle to refer to each other
    RelinkableHandle<YieldTermStructure> intEuribor3m, intEuribor6m;

    // build euribor3m
    // use intEuribor6m in rate helpers
    ...
    auto euribor3m = mc->addCurve(
        intEuribor3m, ext::make_shared<PiecewiseYieldCurve>(...));

    // build euribor6m
    // use intEuribor3m in rate helpers
    ...
    auto euribor6m = mc->addCurve(
        intEuribor6m, ext::make_shared<PiecewiseYieldCurve>(...));

    // any uses external to the cycle should use the external
    // handles: euribor3m and euribor6m
}

[pcaspers]

@eltoder indeed, looks simpler, I'll change this

[pcaspers]

I added a unit test for a cycle that contains a pwyc and a zero spreaded curve. There are two issues still to be resolved:

• I am not able to do multiCurve->addBootstrapObserver(ptrois.get()) because that leads to a crash, apparently caused by a notification cycle between the pwyc and zero spreaded curve. At the moment I don't see why, because the internal handle to pwyc should have been converted to a non-observing handle. The test still works, since ZeroSpreadedTermStructure does not require an update. But that can not be assumed in general.

• I wonder if we want to add intcurveois to MultiCurve similar to what we do with piecewise yc handles in MultiCurve::addCurve() with the only difference that we replace multiCurveBootstrap_->add() with multiCurveBootstrap_->addBootstrapObserver() there. Then we wouldn't need to call multiCurveBootstrap_->addBootstrapObserver() in the client code.

[eltoder]

@lballabio I thought of another option: it's similar to what @pcaspers had previously, but we move multiCurveBootstrapContributor() from the base YieldTermStructure into its own class, say MultiCurveBootstrapProvider. Then we'll just do a dynamic_cast to it in addCurve. We can even arrange that PiecewiseYieldCurve is only inherited from MultiCurveBootstrapProvider if it has a compatible bootstrapper. What do you think? I think it will make the API more symmetric and we can do the move change I mentioned above. I can write it down if that's easier.

[pcaspers]

I think I resolved this:

I added a unit test for a cycle that contains a pwyc and a zero spreaded curve. There are two issues still to be resolved:

• I am not able to do multiCurve->addBootstrapObserver(ptrois.get()) because that leads to a crash, apparently caused by a notification cycle between the pwyc and zero spreaded curve. At the moment I don't see why, because the internal handle to pwyc should have been converted to a non-observing handle. The test still works, since ZeroSpreadedTermStructure does not require an update. But that can not be assumed in general.
• I wonder if we want to add intcurveois to MultiCurve similar to what we do with piecewise yc handles in MultiCurve::addCurve() with the only difference that we replace multiCurveBootstrap_->add() with multiCurveBootstrap_->addBootstrapObserver() there. Then we wouldn't need to call multiCurveBootstrap_->addBootstrapObserver() in the client code.

@lballabio @eltoder looking at the new unit test, the asymmetry in the API becomes particularly obvious

    auto multiCurve = ext::make_shared<MultiCurve>(accuracy);

    auto ptr3m = ext::make_shared<CurveType>(vars.today, helpers3m, Actual360(), LogLinear(),
                                             GlobalBootstrap<CurveType>(accuracy));
    auto curve3m = ptr3m->addToMultiCurve(intcurve3m, multiCurve);

    auto ptrois = ext::make_shared<ZeroSpreadedTermStructure>(intcurve3m, b);
    auto curveois = multiCurve->addNonPiecewiseCurve(multiCurve, intcurveois, ptrois);

Therefore I would like to try the suggestion with MultiCurveBootstrapProvider.

[lballabio]

Sure Peter, go ahead.

[eltoder]

@lballabio @pcaspers it turns out only inheriting from MultiCurveBootstrapProvider when PiecewiseYieldCurve has correct bootstrapper is pretty hard, because Bootstrap class is actually a template parametrized by the curve type, so this leads to a circular reference. There may be a way, but I did not find it yet.

Otherwise it is pretty straightforward. I pushed an example change here: https://github.com/eltoder/QuantLib/commits/multicurve_bootstrap_3/

[pcaspers]

Thanks @eltoder - I think I did something similar in parallel. I use a plain reference instead of rvalue reference in addCurve() and set it to a nullptr. Is there an advantage in passing the rvalue reference? Happy to change it. And are we on the same page regarding addNonPiecewiseCurve()?

[eltoder]

@pcaspers I think rvalue references are nicer: you can pass std::make_shared directly without first assigning to a variable, and if you do have a variable it forces you to write std::move, which makes it obvious that the variable cannot be used afterwards. With lvalue reference you're forced to create a variable, and it's not clear that the variable is mutated by addCurve.

And I apologize, I'm traveling and didn't have a chance to look at your addNonPiecewiseCurve().

[pcaspers]

ok that makes sense, I will switch to rvalue references

[eltoder]

@pcaspers possibly an over-engineering, but I got inheriting from MultiCurveBootstrapProvider only for GlobalBootstrap working. I added a commit to the same branch.

[eltoder]

@pcaspers addNonPiecewiseCurve() makes a lot of sense to me, but I would use a more generic name. Maybe addNonBootstrappedCurve()? Also, the bottom halves of addCurve() and addNonPiecewiseCurve() appear identical, so probably can be factored out into a private helper method.

EDIT: I actually suggested something similar but with different names in an EDIT to this comment: #2344 (comment) :-)

[pcaspers]

@eltoder thank you - how about a more symmetric naming, like addBootstrappedCurve() and addNonBootstrappedCurve()?

I factored out the common code.

Regarding the conditional inheritance, I am happy to include that if we think it is useful. Although the code gets more complicated and harder to read and understand. So I leave that decision to you and @lballabio.

[eltoder]

@pcaspers the naming sounds good to me.

I agree about conditional inheritance. It looks somewhat convoluted and isn't critical. (It is a small optimization: iterative curves will be smaller and we can remove the null check in addCurve. It's also a bit cleaner if all classes that implement MultiCurveBootstrapProvider can actually provide it.) So it is probably not worth it, but let's see what @lballabio says.

[lballabio]

Let's stay simple, at least for now.

[lballabio]

Hmm, I'm not sure I'd force client code to use std::move (which is not possible in Python anyway)

[pcaspers]

Would the plain reference and a move from that reference inside the method work? Plus I guess assigning a nullptr to the input reference to have a defined value in that ptr?

[lballabio]

I guess so, but do we really want to empty the passed shared_ptr?

[pcaspers]

If I understand correctly we wanted to avoid the user sticking that pointer into a handle instead of using the returned handle. And if they do make the code fail early and with a clear error.

[pcaspers]

In order to not loose the neatness of rvalue refs, we could add two overloads with plain and rvalue references?

[lballabio]

Hmm, they might also try to use the pointer without putting it into a handle, in which case they'd get a clear-ish error with boost::shared_ptr but a segfault with std::shared_ptr... I'm not sure we can cover all the bases, we'd have to rely on documentation anyway.

[lballabio]

In order to not loose the neatness of rvalue refs, we could add two overloads with plain and rvalue references?

we could, it depends on what we want. If we do want to empty the pointer, forcing a call to std::move makes it explicit which is good but we'll have to think about Python etc

[pcaspers]

just a thought, would a unique_ptr help on the Python / ... side?

[lballabio]

I'm not sure it's supported by SWIG. Anyway, I think we can leave it as it is. In C++ we force the std::move and we can export the method in SWIG so that the wrappers do the move.

[lballabio]

I would expand on this (what do we mean by internal and external handle, what is the cycle, the need of using std::move etc) after which it looks ready to merge.

[pcaspers]

I added a (hopefully) improved documentation.

[eltoder]

Thank you for working on this, @pcaspers !

[pcaspers]

Thanks for all your help @eltoder