[enhancement] adding csr online support to oneDAL basic_statistics - #3715
[enhancement] adding csr online support to oneDAL basic_statistics#3715icfaust wants to merge 3 commits into
basic_statistics#3715Conversation
|
|
||
| template <typename Float, daal::internal::CpuType Cpu> | ||
| template <daal_lom::Method Value> | ||
| using daal_method_constant = std::integral_constant<daal_lom::Method, Value>; |
There was a problem hiding this comment.
This follows on precedent in kmeans.
| Float, | ||
| oneapi::dal::backend::interop::to_daal_cpu_type<decltype(cpu)>::value, | ||
| Method>() | ||
| .finalizeCompute(daal_partial_obs.get(), |
There was a problem hiding this comment.
required due to limitations in call_daal_kernel_finalize_compute to the default method (I didn't understand this at first, but have verified).
| return (data.row_count_ > 100 || data.column_count_ > 100) && policy.is_cpu(); | ||
| } | ||
|
|
||
| std::vector<csr_table> split_csr_by_rows(const csr_table& table, std::int64_t split_count) { |
There was a problem hiding this comment.
I need help here. I am not well versed in catch2 (only basic knowledge).
There was a problem hiding this comment.
What kind of help?
This implementation looks reasonable from the first glance for CPU-only usage.
There was a problem hiding this comment.
Please move it to the common place: https://github.com/uxlfoundation/oneDAL/blob/main/cpp/oneapi/dal/test/engine/tables.hpp
There was a problem hiding this comment.
Would need to rename this file based on its use for csr support as well (following the precedent of basic_statistics for batch jobs).
2 participants
Description
Will need to iterate, developed with claude follow precedents set by
basic_statsticsandkmeans.Checklist:
Completeness and readability
Testing
Performance