MNT Uses memoryviews in tree criterion #22921
Conversation
There was a problem hiding this comment.
Thank you, @thomasjpfan.
Relying on numpy's allocator is indeed more appropriate.
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
That's expected. memset natively works on blocks that have the size of the registers. The loop can't compete with that, even more when not all optimisation flags are enabled. |
I reran the benchmarks for entropy again with memoryviews (repeating it 30 times with different random seeds), and I got similiar results between From memory, my original benchmarks showed a ~1% runtime regression compared to main. Maybe something was running on my system during my original benchmarks. If you are interested in running the benchmark: python benchmark.py pr_results.json --config entropywill store the results in |
|
@jjerphan you might want to take another look at it ? |
sklearn/tree/_criterion.pyx
Outdated
| @@ -319,8 +292,7 @@ cdef class ClassificationCriterion(Criterion): | |||
| cdef SIZE_t offset = 0 | |||
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
Continues #22868
This PR refactors
sklearn/tree/_criterion.pxdto use memoryviews. This PR simplifies the code because strides are handled by the memoryview and Python can handle the memory. I ran this benchmark with every criterion and different n_samples and noticed no change in performance. Here are the plots of the results and here is the raw results on main and raw results for this PR.Note that
memcpyandmemsetare still used because they benchmark better when compared to their memoryview counter parts (mv[:] = 0.0ormv[:] = other_mv).