meerk40t / svgelements

SVG Parsing for Elements, Paths, and other SVG Objects.
MIT License
133 stars 29 forks source link

Optimize CubicBezier.length() with scipy if available #58

Closed abey79 closed 3 years ago

abey79 commented 3 years ago

A new scipy-based implementation of CubicBezier.length() is added and used if scipy is available. A new test checks that the result of the scipy and default implementation match.

Simple benchmark:

In [4]: p = svgelements.CubicBezier((3, 4), (324, 12), (32, 234), (123, 456))

In [5]: %timeit p._length_default()
1.44 s ± 5.04 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [6]: %timeit p._length_scipy()
2.5 ms ± 143 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

Due to how slow the current implementation is, this test currently increase execution time by more than a 1 minute on my computer. Should I reduce the number of iteration?

Fixes #55

tatarize commented 3 years ago

I'll check it. But, it looks great. I might prefer catching ImportError rather than ModuleNotFoundError

https://www.scivision.dev/python-importerror-vs-modulenotfound/

abey79 commented 3 years ago

Oh ok, I didn't know about ImportError vs. ModuleNotFoundError -- I'll fix it tomorrow morning.

Should we make a note somewhere that this was directly copy/pasted from svgpathtools?

tatarize commented 3 years ago

It's extremely minor. You'd need to have scipy but have it broken, So it found the module but then couldn't actually load it. So it throws the other type of error.

AssertionError: 58.67678805630189 != 58.67678799098887 within 7 places (6.531301721679483e-08 difference)

The error for almost equal might need to be increased there. The odds are that the linear version is broken here rather than the improved version. As for the count I think I made it the Arc one that takes forever take literally 2 as the same size since it's still a long test.

I'll likely merge this as is and then just change the import and give the test a wider margin since the current length code with lines is likely a lot more able to allow errors creep in well below margin.

tatarize commented 3 years ago

Yeah, confirmed the last little bit I needed. The errors at 1e-6 error linear averages around 3.3e-05. When linear error is put up to 1e-12 it drops to about 3.4e-9, so the error is clearly on the slower linear method. Which is all generally expected since it's largely the same as the svgpathtools stuff and any problem there would have been discovered already.