Conversation
|
It looks like a bunch of issues come up with this design from the tests, I just haven't run into any of them in my usage. Regardless, now this PR is here you can see what I'm doing and potentially help with the change (I only have so much time I can spend on this). |
|
@ablaom I've found this codebase a little awkward to work with 😅 but I've managed to resolve the merge conflicts. It would be good if someone else could take a look at this and help move it forwards. |
|
@ablaom I'm guessing you don't have much time to spend on this? |
|
I don't doubt that you have made valuable progress here. However, as my time is limited, my modus operandi must be to only review PRs that pass tests, I'm afraid. |
|
I feared that that might be the case. Unfortunately I don't think I have the time to work this out by myself either. I guess we can hope for a helpful third party to come along at some point. The ~10x prediction performance improvement would be rather nice to have. |
This adds the change mentioned in #159, removing the (quite large) performance discrepancy between predicting with and without probability. I'd like to see the evaluation of decision paths be improved, but I haven't had the time to really dive into that so I may as well just upstream this improvement for now.
In testing I've seen a ~10x improvement in prediction time, and similar improvements in memory usage. See https://tecosaur.com/public/treeperf.html#observation-1 for more information.