-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
td_seq perf improvement #774
Conversation
Thanks for the PR with the speed improvements. Updating it and a few other indicators and methods have drastically improved the overall speed of the library. 😎 On another note, TradingView (TV) has pulled down and closed sourced all public DeMark indicators due to trademark™️ reasons. To get ahead of the issue, I modified it some and named it exhc as we do not truly know what the correct source is and thus not able to validate it. Lastly in the updated development branch, I incorporated a numba version of wma to speed it up as well around the same time you submitted #776. Curious about the speed differences you find between the one I updated and the recent one you submitted. KJ |
Nice work @twopirllc on To answer the question regarding the Numba version of In the test runs I did, I can see consistent .001s calls to |
Yeah probably not for wma (et al). It was just something I whipped up while working on ht_trendline since it was also using wma. I will implement the best calculation for job in an upcoming push.
Since incorporating numba, I decided that core calculations should be done with numpy, demse loop calculations with numba and when necessary pandas functions.
I hear you. I contemplated a long time whether or not to require numba. Originally the library was to have few dependencies (just pandas and it's dependents), but demands have required a bit more. Undoubtedly requiring numba will increase container size, but I think the speed improvements alone exceeds those concerns. Though I wish there was more insight on user environments, but I assume, maybe incorrectly, that many users may already have numba in their dependency chain already. 🤷🏼♂️ Yeah I am not thrilled with the initial jit compilation either but it is a minor initial inconvenience. I just do a dry run with 200 bars to initialize and test that all the indicators ran. Numba does have Ahead-of-Time (AOT) compilation that maybe become depreciated and does have some caveats. But unfortunately have not had the opportunity to test and determine it's feasibility for inclusion. 😅
Nice. How many bars did you test with? Did you test via a terminal or in a Notebook? Curious about the difference between my M1 an your M2? |
I just checked Numba size on disk, it is only 23MB including the binary blobs of compiled code that were created when I was doing my testing. So maybe increase container sizes isn't an issue - what's another 23MB hey? The testing setup is very rudimentary. Just a Python script. I added a new function in the overlap package
I used both AirBnb EOD prices which is only 3 years and also IBM which has over 15K rows dating back to 1962. There was no real diference in the performance of the functions with AirBnB data and IBM, both were basically the same speed |
I'm not familiar with this particular indicator, but the slowness was caused by the use of apply function on the rolling window in
calc_td()
Using the Pandas native functions
cumsum()
andwhere()
should give the same results. The test data I used confirmed it was the same, but not sure if needs additional test cases.Original performance as reported by cProfile
Updated code performance