domokane / FinancePy

A Python Finance Library that focuses on the pricing and risk-management of Financial Derivatives, including fixed-income, equity, FX and credit derivatives.
GNU General Public License v3.0
2.15k stars 321 forks source link

Major implementation change for ibor_swap? A notebook example doesn't tie out anymore. #188

Closed YimingZhang07 closed 1 year ago

YimingZhang07 commented 1 year ago

Hello,

I looked at the ibor_swap implementation. And noticed an example in FINIBORSWAP_ComparisonWithQLExample.ipynb, which I believe was trying to convince the results tie to the Quant Lib implementation.

This notebook ran with version 0.184, and the results matched perfectly. However, with the latest version, the difference enlarged significantly.

The fixed leg has no problem. The floating leg was 2292441.20, but now it turns to be 2318923.97 mainly due to the change in rate calculations on index curve. The change seems nontrivial.

I am not sure if this notebook should be updated, and most importantly, we could not tie anymore, so either one must have a problem.

BTW, the print_valuation function in swap_float_leg instrument will call print_payments as well, creating visual verbose including duplicate tables and info, maybe this could be improved this as well. (this is also a new problem in latest version).

Thanks,

domokane commented 1 year ago

Thanks. I will look into this. I have not had time to push the most recent changes. Maybe this weekend.

nashquant commented 1 year ago

Hi Professor, I've probably introduced the changes related to the swap legs, so happy to pick up this review this weekend if you are ok with it.

domokane commented 1 year ago

OK. That would be good thanks.

------ Original Message ------ From "Matheus Pires" @.> To "domokane/FinancePy" @.> Cc "domokane" @.>; "Comment" @.> Date 07/07/2023 17:39:21 Subject Re: [domokane/FinancePy] Major implementation change for ibor_swap? A notebook example doesn't tie out anymore. (Issue #188)

Hi Professor, I've probably introduced the changes related to the swap legs, so happy to pick up this review this weekend if you are ok with it.

— Reply to this email directly, view it on GitHub https://github.com/domokane/FinancePy/issues/188#issuecomment-1625597177, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ73PLRXZTSWVCJSCNUJUTXPAUSTANCNFSM6AAAAAA2A7LDYA. You are receiving this because you commented.Message ID: @.***>

nashquant commented 1 year ago

Alright, I started looking into this.

1) Formatting: Indeed the choice I made to include print_payments within print_valuation wasn't good, what if I refactored the valuation table schema like the image below + remove print_payments from it? I.e. Upside: Quite concise table, with all relevant info / Downside: User would need to print_payments to check Start/End Accrual valuation dates, otherwise the table would be too long to display. Let me know if you have a better idea!

image

2) Values not matching: I suspect that this isn't related to my change, but of course gonna keep debugging this. Looking at the notebook @ version 0.184, noticed the column "Rate/IBOR" used to derive the cashflows are quite different (e.g. comparing the two images, for 1st payment, it is 2.005 in one,and 1.97758 in the other, which explains the final difference). Nevertheless, the logic to derive the rates shown wasn't changed in my PR, @domokane are you aware of any change that might be related to this?

image

I'll take a look again later

Cheers, Matheus

nashquant commented 1 year ago

Apparently the float_leg issue was introduced in commit 2d43d10780dcb9afbcefee805c2660c26da08ab9.

If you look at the changes in swap_float_leg.py, you'll notice fwd_rate calculation changed to use index_alpha instead of pay_alpha, which explains the IBOR Rate change I highlighted above.

Professor, do you recall why this was necessary?

PS: For the 1st payment in the example above, we have: Pay_alpha == 0.2555555 Index_alpha == 0.2520547

Which is explained by Index being in Act_365F day count, whereas Pay_alpha is ACT_360 - the two account for 92 days of accrual.

Thanks

domokane commented 1 year ago

Hi - I believe I did this as it seemed appropriate to separate the choice of accrual method for the index from the choice of accrual method for the payment leg. Perhaps I have mispecified the index accrual method in the example - it should be the same as the payment method ? Would that fix the problem ?

nashquant commented 1 year ago

Hi @domokane, thanks for your reply.

nashquant commented 1 year ago

After giving it some thought, I'm quite confident that the updated value for the float leg is the right one - the changes you've made look good to me. Besides that, considering that we're setting Libor rate at a flat 2%, the figures above tell us that in version 0.184 (which matches the results in the blog), the Libor rate was off a bit, coming in at 1.97%.

domokane commented 1 year ago

OK. Thanks.

nashquant commented 1 year ago

Thank you, when I find sometime I'll create a PR for the formatting fix.

YimingZhang07 commented 1 year ago

Thanks for looking at this. If we confirm the current methodology is right, I strongly recommend refreshing the related notebooks too see the impacts if possible. As I see, the valuation difference can be as large as 15-20% for example in FINIBORSWAP_DefiningAFixedFloatingSwap.ipynb. What's more, since swap is fundamental in building curves, e.g. in CDS valuation, I wonder if our results could still tie with Markit.

I am willing to help on this as well, but in coming weeks.

domokane commented 1 year ago

That's fine. I will look at it too. I have some deadlines but in a week or two I should have time.

------ Original Message ------ From "YimingZhang07" @.> To "domokane/FinancePy" @.> Cc "domokane" @.>; "State change" @.> Date 10/07/2023 16:02:32 Subject Re: [domokane/FinancePy] Major implementation change for ibor_swap? A notebook example doesn't tie out anymore. (Issue #188)

Thanks for looking at this. If we confirm the current methodology is right, I strongly recommend refreshing the related notebooks too see the impacts if possible. As I see, the valuation difference can be as large as 15-20% for example in FINIBORSWAP_DefiningAFixedFloatingSwap.ipynb. What's more, since swap is fundamental in building curves, e.g. in CDS valuation, I wonder if our results could still tie with Markit.

I am willing to help on this as well, but in coming weeks.

— Reply to this email directly, view it on GitHub https://github.com/domokane/FinancePy/issues/188#issuecomment-1629032331, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ73PK2GSLORQFIZMTD64DXPQDPRANCNFSM6AAAAAA2A7LDYA. You are receiving this because you modified the open/close state.Message ID: @.***>