-
Notifications
You must be signed in to change notification settings - Fork 2.1k
add instrument weights to global bootstrap #2398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I think this is not the right way to go about this. In general, for global bootstrap we can assign weights to benchmarks. There are many ways to come up with weights, generally based on liquidity and "importance", so weights should be provided by the user. This is how we deal with futures in our code -- we scale their weights by 1e-2. Currently, global bootstrap does not allow passing custom weights -- all quote errors get a weight of 1. (Partially because of this we pass an empty list of helpers into the curve and add everything via additionalHelpers and additionalPenalties.) I think it would be good to allow passing custom weights into the bootstrapper. This is a generic and backwards compatible change that gives a lot of flexibility. You can also use it to solve your issue with normalizing futures vs swaps. If we think that this is not good enough and want a more out-of-the-box experience, I think we should change quoteError(). We could just add a scale to the base class and multiply the difference inside of quoteError() like you are suggesting in the comment. But a more generic thing to do is to make quoteError virtual. Then we can override it for futures to just return values in the rates space directly. I think this is cleaner. Other notes on your change:
|
|
Thanks for your feedback @eltoder. The changes were not meant to be complete. I just wanted to sketch one possible approach to start the discussion. You are right, what I did is not consistent, I wanted to scale all quote errors to order 1, i.e., to scale the swap quote error by 1E2, but leave the future quote error untouched. Anyway, I agree that user-provided weights are more flexible, and I think we don't need to normalize the orders of the quote errors then. I can make this change in this branch here. And agreed on the IterativeBootstrap, this is independent of the scale of the errors. |
|
If you want to scale swaps, you'll also need to scale deposits, fras, etc? I think there are fewer helpers for futures (just regular and overnight), so scaling futures seems easier. But in any case, if you go with custom weights in global bootstrap, everyone can do their own thing. |
|
Yes, indeed. The custom weights are a much better approach. |
| template <class Curve> void GlobalBootstrap<Curve>::initialize() const { | ||
|
|
||
| // ensure helpers are sorted | ||
| std::sort(ts_->instruments_.begin(), ts_->instruments_.end(), detail::BootstrapHelperSorter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do weights work with this sort? If we didn't have transparency on this implementation, it would seem natural to pass a vector of weights corresponding to the vector of instruments passed to the curve (as in, weights[i] corresponds to helpers[i]), but this seems to require that the weights correspond to the instruments after they go through sorting (and if that's the case, we'd need std::stable_sort here or we wouldn't be able to predict the order if two maturities are the same).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, good catch. I'll fix it. The sorting of the weights should correspond to the original instrument sorting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the sorting, I do not think it is necessary for the global bootstrap.
While reading through the code I noticed a potential issue with the underlying interpolation: This should cover the range from t = 0 to the maximum of the last relevant dates of the given helpers. However, the times vector used to build the interpolation contains the pillar dates of the helpers, which might be earlier. I guess we should enable extrapolation on the interpolation instance therefore to cover the general case?
I am slightly confused about this point, since the same issue seems to apply to IterativeBootstrap, i.e., how can we retrieve the quote error from a helper if its pillar date is earlier than its last relevant date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ignore my last comment. Whether extrapolation is allowed is not controlled by the interpolation object, but by the TermStructure and maxDate(), maxTime(). maxDate_ is set explicitly to the maximum of all last relevant dates. In addition, the extrapolation does not use the interpolator anyway, but assumes a flat instantaneous forward rate.
So I'll do something similar in the global bootstrap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In effect, we fix the case where the additional dates do not cover the requirements of the additional helpers w.r.t. their latest relevant dates.
resolves #2396
Question:s How backwards compatible do we want to be, i.e.