feature: oneMKL migration #2865
Conversation
Alexandr-Solovev
requested review from
Alexsandruss,
samir-nasibli,
napetrov,
homksei,
ahuber21 and
ethanglaser
as code owners
|
@Alexandr-Solovev You'll need to work on that public CI still uses mircomkl (so far seeing no change there), and please include an informative description (or is this still supposed to be in draft?) |
|
@icfaust Hi! Yes, sorry its a draft, but thanks for the comment about WORKSPACE |
|
/azp run CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Question: do we need to update |
There was a problem hiding this comment.
Huge work done. Thank you @Alexandr-Solovev !
I am sure we should have follow up tickets for all TODOs.
Provided just minor comments.
For merging, I recommend getting approval from other reviewers as well.
| @@ -87,16 +72,23 @@ mkl_repo( | |||
| name = "mkl", | |||
| root_env_var = "MKLROOT", | |||
| urls = [ | |||
| # TODO: when the issue with binutils will be solved, replace 2023.0 to 2024.2 | |||
There was a problem hiding this comment.
Please cover this with a ticket
| memcpy(dest, src, srcSize); | ||
| return 0; | ||
| // TODO: safe funtion | ||
| // return memcpy_s(dest, destSize, src, srcSize); |
There was a problem hiding this comment.
Please add some ticket for this kind of refactoring
|
|
||
| static float serv_string_to_float(const char * nptr, char ** endptr) { return __FPK_string_to_float(nptr, endptr); } | ||
| // TODO: not a safe function - no control for the input buffer end |
| __DAAL_VSLFN_CALL(fpk_vsl_sub_kernel, vsliSSEditTask, (task, __DAAL_VSL_SS_ED_CP_STORAGE, &cpStorage), errcode); | ||
| __DAAL_VSLFN_CALL(fpk_vsl_sub_kernel, vsldSSEditTask, (task, __DAAL_VSL_SS_ED_ACCUM_WEIGHT, weight), errcode); | ||
|
|
||
| ThreadingFuncs threading = { _daal_mkl_threader_for, _daal_mkl_threader_for_ordered, _daal_mkl_threader_sections, _daal_mkl_threader_ordered, |
There was a problem hiding this comment.
I guess this far somehow discussed, make sense for reply comment with the answer before marking it resolved
| namespace mkl | ||
| { | ||
| //It's a placeholder, the real function calls exact in xfunctions. | ||
| //TODO: add correct threading control |
|
intel/scikit-learn-intelex#2066 is merged. |
Description:
Feature: Migration from FPKs to oneMKL
Summary:
This PR updates the oneDAL codebase to ensure compatibility with oneMKL. It includes various fixes and modifications across multiple files to address issues related to the integration with the new libraries.
Key Changes:
Updating Public CI Environment:
Removing macOS Public CI Step:
Bazel Changes:
Service Functions:
Naming Updates:
Macros Usage and CPU Instructions Updates:
DAAL_MKL_CALLmacros have been replaced withfname_fargsmacros to call the equivalent functions in oneMKL.Sparse API Updates:
PCA Algorithm Refactoring:
Makefile Updates:
Known Issues/Looking for Help:
1. Syevd and Gesvd Tests:
2. Public Build for Windows (WindowsMakeVC Step):
3. Removing Unnecessary CPU Instructions and Macros:
4. Fixing Path Substitution for Windows:
\,/, and$for proper Windows path handling.