lballabio / QuantLib-SWIG

QuantLib wrappers to other languages
Other
339 stars 282 forks source link

Implement `Date.to_date` and `from_date` in C++ #608

Closed eltoder closed 7 months ago

eltoder commented 7 months ago

This makes them about 3x faster.

lballabio commented 6 months ago

@eltoder, a heads-up: I'm probably reverting this so that we can use Python's limited API. We're using 1.1 Gb on PyPi for each release, which is not sustainable. Using the limited API we can cut that by more than half. Unfortunately, it doesn't support the PyDate_* functions. We'll have to research some other way to write this from C++.

eltoder commented 6 months ago

@lballabio I see. Did you check performance impact of switching to limited API? It can be non-trivial.

It's not very hard to rewrite this without using PyDate functions: replace PyDateTime_IMPORT with a PyImport_ImportModule("datetime") and use other generic PyObject APIs. I can do this if you want. It will be slower of course.

eltoder commented 6 months ago

Btw, at least half of the size of the wheel is coming from the bundled QuantLib.so, which does not depend on the Python version. I wonder if it's possible to separate it into some sort of a private package without Python version dependency and make the public wheels depend on it. This can also cut the size in half without any performance impact.

lballabio commented 6 months ago

Did you check performance impact of switching to limited API? I can be non-trivial.

I just did. I wrote a test script in which I bootstrap a curve over a bunch of quoted OIS rates and then reprice a bond for each business day over a period of 7 years. Running it 5 times with timeit gave me these times for the current 1.33 wheel...

[25.2165666250512, 25.441410708008334, 25.272593833040446, 25.483433542074636, 25.492740332963876]

...and these for the abi3 wheel:

[25.31827799999155, 25.53022258298006, 25.54050083400216, 25.55305774998851, 25.54700454196427]

so it doesn't look like a huge problem.

eltoder commented 6 months ago

Nice. I'll send a PR to make this work with Limited API.

lballabio commented 6 months ago

Thanks!