-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Include time zero / evaluation date in option date interpolation #1783
Include time zero / evaluation date in option date interpolation #1783
Conversation
Looking a bit closer I notice that for quite a few |
@lballabio I'll check the failing test cases with high resolution dates - however I'll wait for your confirmation that we want to do this "intermediate" fix. |
Ouch, this slipped under the radar. I'm ok, I guess? I'd just keep |
I think the code mixes up the evaluation date from the settings with the reference date from the term structure. I fixed that, but renamed the protected member |
The 3 new issues in the "Codacy Static Code Analysis" check look like false positives. Can those be added to an ignore-list somewhere? |
registerWith(Settings::instance().evaluationDate()); | ||
evaluationDate_ = Settings::instance().evaluationDate(); |
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.
This is because it's already done in the base class, right? We still need a notification if the evaluation date changes, don't we?
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.
Yes I think it is done in the TermStructure
constructor.
I would hope derived classes are not using it. Let's change it, I'll document the possibly breaking change in the release notes. I don't see an easy way to rename it in a backward-compatible way. |
Sounds good. So we are done here or is there something left to do for me? |
Nope, I'm merging. |
…olation ql pr is here: lballabio#1783
For times before the first option date
optionDateFromTime()
can return a date < evaluation date. I would suggest to include the evaluation date and t = 0 into the linear interpolation used to do the time to date conversion.