Skip to content
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

Computation of cdf in LocalVolRNDCalculator #1435

Merged
merged 5 commits into from
Sep 7, 2022
Merged

Computation of cdf in LocalVolRNDCalculator #1435

merged 5 commits into from
Sep 7, 2022

Conversation

mdotlic
Copy link
Contributor

@mdotlic mdotlic commented Jul 22, 2022

Computing cdf boundaries was wrong. In case xl is positive, while loop lowers xl but only to 0, if it is negative xl (through while loop) rise towards 0. In case xr is positive previous code is good as it tends towards +infinity. However, if xr is negative it tends towards -infinity.

xl should always tends towards -infinity and xr towards +infinity (up to a tolerance)

Maybe this can be done in better way, however this should work

Computing cdf boundaries was wrong. In case xl is positive, while loop it lowers it but only to 0, if it is negative xl (through while loop) rise towards 0. In case xr is positive previous code is good as it tends towards +infinity. However, if xr is negative it tends towards -infinity.

xl should always tends towards -infinity and xr towards +infinity (up to a tolerance)

Maybe this can be done in better way, this is the first thing that come to my mind
Update localvolrndcalculator.cpp
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 22, 2022

Thanks for opening this pull request! It might take a while before we look at it, so don't worry if there seems to be no feedback. We'll get to it.

@CLAassistant
Copy link

CLAassistant commented Jul 22, 2022

CLA assistant check
All committers have signed the CLA.

@mdotlic mdotlic marked this pull request as ready for review July 22, 2022 08:54
@klausspanderen
Copy link
Contributor

Hi

thanks for spotting this issue. Maybe we should initialize the addition factor with a relative scale instead of an absolute value? E.g.

Real addition = 0.1*(xr-xl);

cheers
Klaus

@lballabio
Copy link
Owner

Thanks, @mdotlic! May you check the automated comment above by CLAassistant and click the link?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 71.174% when pulling b76d14e on mdotlic:master into 63bac2f on lballabio:master.

@lballabio lballabio added this to the Release 1.28 milestone Sep 7, 2022
@lballabio lballabio merged commit 3a4205b into lballabio:master Sep 7, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 7, 2022

Congratulations on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants