lballabio / QuantLib

The QuantLib C++ library
http://quantlib.org
Other
5.43k stars 1.8k forks source link

Fix lower/upper bounds for global bootstrap #2120

Closed eltoder closed 2 days ago

eltoder commented 3 days ago

Currently global bootstrap uses minValueAfter/maxValueAfter methods from Traits to come up with lower/upper bounds. This is problematic. These methods depend on the previous point in the curve. In the iterative bootstrap these methods are called after the previous point is already known exactly. However, in the global bootstrap all points are still filled with Traits::initialValue. This makes the bounds too tight and leads to bad results for curves with higher interest rates.

Instead add separate minValueGlobal/maxValueGlobal methods that do not rely on previous data.

eltoder commented 3 days ago

CC @pcaspers as the original author of the code

coveralls commented 3 days ago

Coverage Status

coverage: 73.107% (+0.003%) from 73.104% when pulling c987e2d9f42adac890aeb2fad85db9af7741cbbf on eltoder:feature/global-bootstrap-bounds into ce96c8e67392eff8a274b1d24ece3e7defe0e8ea on lballabio:master.

lballabio commented 3 days ago

It seems like calling minValueAfter/maxValueAfter with validCurve=false would have the same result, wouldn't it? It would be less explicit but it would remove some duplication and avoid the need to update user-defined traits.

eltoder commented 3 days ago

It wouldn't for the Discount trait, which is the most commonly used one AFAIK. It uses the previous point when validData==false: https://github.com/lballabio/QuantLib/blob/ce96c8e67392eff8a274b1d24ece3e7defe0e8ea/ql/termstructures/yield/bootstraptraits.hpp#L92 https://github.com/lballabio/QuantLib/blob/ce96c8e67392eff8a274b1d24ece3e7defe0e8ea/ql/termstructures/yield/bootstraptraits.hpp#L101

We'd need to introduce a new flag like usePrevious or abuse the currently unused firstAliveHelper. Other options are to add a check like

return c->data()[i-1] == 1.0 ? maxDF : c->data()[i-1] * std::exp(detail::maxRate * dt);

or to just change the code to return constants (0 and maxDF) even for the iterative bootstrap. We rarely use iterative bootstrap, so I don't know how valuable this calculation is for its performance.

All of this seems less clean than new methods, especially since the comment says pretty explicitly

// possible constraints based on previous values

For the global bootstrap I need methods that do not depend on previous values. However, let me know what your preference is.

lballabio commented 2 days ago

I see, thanks. I missed the difference in the discount traits.

eltoder commented 2 days ago

Thank you. I do agree that this causes a bit of duplication. Also, C++ doesn't have a nice way to make an alias when the logic is the same (e.g. minValueGlobal = minValueAfter). If you come up with a better way to do this, I'm happy to change it.