-
Notifications
You must be signed in to change notification settings - Fork 211
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
MAINT: remove DAAL CL kernels and GPU interfaces #2816
MAINT: remove DAAL CL kernels and GPU interfaces #2816
Conversation
/intelci: run |
/intelci: run |
/intelci: run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used grep on cpp/daal and showed the following SYCL/sycl mentions in your branch:
cpp/daal/include/services/internal/buffer.h: * @ingroup sycl
cpp/daal/src/services/error_handling.cpp: // Group of SYCL-related errors -90900..-90999
cpp/daal/include/services/daal_defines.h:#if (defined(__INTEL_COMPILER) || defined(__INTEL_LLVM_COMPILER)) && !defined(SYCL_LANGUAGE_VERSION)
cpp/daal/include/services/daal_defines.h:const int SERIALIZATION_SYCL_SOA_NT_ID = 3500;
cpp/daal/include/services/daal_defines.h:const int SERIALIZATION_SYCL_CSR_NT_ID = 3503;
cpp/daal/include/services/daal_defines.h:const int SERIALIZATION_SYCL_HOMOGEN_NT_ID = 7500;
cpp/daal/include/services/error_indexes.h: // Group of SYCL-related errors -90900..-90999
cpp/daal/include/services/internal/buffer.h: * \brief Wrapper for a SYCL* buffer
cpp/daal/include/services/internal/buffer.h: * or on host/device sides using SYCL* buffer
cpp/daal/include/services/internal/buffer_impl.h: * <a name="DAAL-CLASS-SERVICES-INTERNAL__SYCLBUFFERIFACE"></a>
cpp/daal/include/services/internal/buffer_impl.h: * \brief Common interface for SYCL*-backed buffer
Due to the size, my initial review is sort of shallow. Sorry. You can respond with those that should stay and why.
...algorithms/dtrees/gbt/regression/gbt_regression_train_dense_default_batch_fpt_dispatcher.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me! Thanks! I added a couple points related replacement sycl containers, probably its not necessary. And also, please take a deep look on CI, probably makes sense to launch nightly on your pr instead of intelci:run
...al/src/algorithms/dtrees/forest/regression/df_regression_train_hist_batch_fpt_dispatcher.cpp
Show resolved
Hide resolved
...gorithms/dtrees/gbt/regression/gbt_regression_predict_dense_default_batch_fpt_dispatcher.cpp
Show resolved
Hide resolved
cpp/daal/src/algorithms/k_nearest_neighbors/bf_knn_classification_model_impl.h
Show resolved
Hide resolved
...hms/k_nearest_neighbors/bf_knn_classification_predict_dense_default_batch_fpt_dispatcher.cpp
Show resolved
Hide resolved
cpp/daal/src/algorithms/k_nearest_neighbors/bf_knn_classification_train_kernel_impl.i
Show resolved
Hide resolved
cpp/daal/src/algorithms/kmeans/kmeans_init_dense_random_distr_step1_fpt_dispatcher.cpp
Show resolved
Hide resolved
Impressive PR, looks good overall. Additional question on
In theory with removal of openCL we don't need openCL -> SYCL compatibility level anymore and could relay on SYCL compiler handling of L0/openCL instead of us loading libs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code conflict solution and rebase are required to get last sklearnex building change.
Correct, I re-added openCL mentions from original removal since it led to issues with unresolved FPK symbols but I am watching those tickets and will create a (much smaller) follow-up to this one once those can be removed |
Description
Removes all openCL kernels and DAAL GPU functionality. Changes consist of:
Can be taken out after merge of intel/scikit-learn-intelex#1770