-
Notifications
You must be signed in to change notification settings - Fork 12
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
Adding CDS bootstrapping #18
Conversation
Added: New classes to contain CDS, survival probabilities, and hazard rates curves New classes to contain CDS curve specifications Added generic methods to format and print for the above classes Created bootsrapping generic methods
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.
Questions:
- Is there a nice way that you can combine the
HazardCurve
andSurvivalCurve
classes into one? Looks like the only reason for separating them is becausecredule::bootstrapCDS()
returns hazard rates and survival probabilities in one object and you want to distinguish them. Possibilities:- can you extend
CDSCurve
by includinghazard_rates
andsurvival_probabilities
fields which are set toNA_numeric
if the instance is not bootstrapped and otherwise the bootstrapped values? OR - define a class called
BoostrappedCDSCurve
which includeshazard_rates
andsurvival_probabilities
fields (plus others) which is returned bybootstrap()
where the latter is a generic that replacesbootstrap_survprob()
andbootstrap_harzardrate()
.
- can you extend
- Where do the valid rank labels come from? Would be better if we could make these nicer labels that have a consistent convention. All lower case would be better (the user doesn't have to remember). For eg. Senior -> "senior", SubTier3 -> "sub_tier_3", SubTier2Upper -> "sub_tier_2_upper", SubTier2Lower -> "sub_tier_2_lower", SubTier1 -> "sub_tier_1"
Actions:
- I have added a few commits to yours on the
imanuelcostigan:feature-cds-ic
branch. (Long story about why I did this). Can you merge these into your fork? Happy to show you how to do this. - Please review my commit commentary to understand my changes.
- Update your version of roxygen2
- Use the print / format pattern of the
ZeroCurve
class for the various credit curve classes you have introduced (main thing is that it goes via a tibble). - Document whether the
probabilities
passed toSurvivalProbCurve
are cumulative or marginal survival probabilities - Review the CONTRIBUTING file on the repo as it relates to PR and make sure you have added test cases, updated the NEWS file, etc
- Update the README.Rmd (and knit the corresponding README.md file)
- Create a vignette for the credit curve functionality
- Ensure there are no errors, warnings or notes returning from
devtools::check()
Added: New classes to contain CDS, survival probabilities, and hazard rates curves New classes to contain CDS curve specifications Added generic methods to format and print for the above classes Created bootsrapping generic methods
* Removed duplicate SubTier1 value * Ordered ranks
* Use inheritParams to avoid repeating text * Export constructor
* Use lowercase argument names (LGD -> lgd) * Assert that tenor and spread arguments to have same lengths (no need to recycle vector values to ensure equivalent lengths). * Improve documentation of CDSCurve * Improve style of code and documentation
* using methods from `assertthat` * ensure that tenors and probabilities are the same length
Prefer not to have concatenated words in method / class names
...like functions for other classes
* Export them * remove newline from format method (will introduce newline twice when called from print() which is typically how this method is called)
1- In this iteration, the new credit classes mirror interest rate classes in functionality, as follows: 2- I have limited the use of Credule package (for bootstrapping) to only one generic: as_SurvivalProbabilities.CDSCurve 3- CreditCurve constructor only needs Survival Probabilities, so it doesn't have any dependency on the bootstrapping. I chose the name CreditCurve instead of the suggested BootstarppedCDSCurve, because the class no longer depends on bootstrapping. 4- Zero Hazard rates are now calculated from Survival Probabilities, without having to do an additional bootstrapping. (This is to ensure internal consistency in terms of business conventions) 5- I have extended all of the interpolation generic functions, to mimic similar functionalities for the credit classes. Here, note that 6- There are additional rules for CDSSpecs in cred-ops. When output specs are ambiguous, they are replaced with an "Empty" CDSSpecs object. 7- for +,-,/ and *, I included an assertion to check if the length of objects are equal. In IR and DF operators, if the length are not equal, equally lengthed vectors are created. I suggest including an assertion for IR and DF operators as well. Point-to-point responses to comments:Question 1- In this iteration, Credit Curve holds that responsibility, in the same way that ZeroCurve does for rates. |
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:
- refresh your PR from master and resolve conflicts
- make sure all checks pass with no warnings, notes etc
Please review the pull request. I have |
In this commit I have
All of the new classes and methods contain full documentation with examples
@imanuelcostigan