Skip to content

Rename USE_ONEMKL as USE_ONEMKL_XPU and set it as default ON #1642

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

Merged
merged 3 commits into from
May 22, 2025

Conversation

CuiYifeng
Copy link
Contributor

@CuiYifeng CuiYifeng commented May 7, 2025

  • USE_ONEMKL (for oneMKL XPU) may be confused with USE_MKL (for oneMKL CPU). New name USE_ONEMKL_XPU provides better readability.
  • Set USE_ONEMKL_XPU as default ON.

@CuiYifeng CuiYifeng requested a review from chuanqi129 May 7, 2025 13:48
@CuiYifeng CuiYifeng marked this pull request as draft May 8, 2025 01:44
@CuiYifeng CuiYifeng force-pushed the yifeng/rename_mkl_env branch from 761ba29 to 88de14e Compare May 16, 2025 03:51
@CuiYifeng CuiYifeng changed the title Rename USE_ONEMKL as USE_ONEMKL_XPU for better readability Rename USE_ONEMKL as USE_ONEMKL_XPU and set it as default ON May 16, 2025
@CuiYifeng CuiYifeng marked this pull request as ready for review May 16, 2025 04:02
@Copilot Copilot AI review requested due to automatic review settings May 16, 2025 04:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR renames the preprocessor macro from USE_ONEMKL to USE_ONEMKL_XPU for clearer differentiation between oneMKL XPU and oneMKL CPU, and sets the new macro as the default ON option.

  • Update macro conditionals in xpu/mkl-specific source files
  • Update build workflow configuration to reflect the renaming

Reviewed Changes

Copilot reviewed 3 out of 7 changed files in this pull request and generated no comments.

File Description
src/ATen/native/xpu/mkl/SpectralOps.cpp Updated macro conditionals to use USE_ONEMKL_XPU
src/ATen/native/xpu/SpectralOps.cpp Updated macro conditionals in xpu FFT functions to use USE_ONEMKL_XPU
.github/workflows/_windows_ut.yml Updated workflow configuration to set USE_ONEMKL_XPU to 1
Files not reviewed (4)
  • .github/scripts/build.sh: Language not supported
  • cmake/ONEMKL.cmake: Language not supported
  • src/BuildOnLinux.cmake: Language not supported
  • src/BuildOnWindows.cmake: Language not supported

@CuiYifeng CuiYifeng requested a review from fengyuan14 May 16, 2025 04:03
@CuiYifeng
Copy link
Contributor Author

@chuanqi129 @fengyuan14 Please help review, thanks.

@@ -50,7 +50,7 @@ git submodule sync && git submodule update --init --recursive
python -m pip install -r requirements.txt
python -m pip install mkl-static mkl-include
export USE_STATIC_MKL=1
export USE_ONEMKL=1
export USE_ONEMKL_XPU=1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any simple API can check whether the oneMKL is compiled and enabled? If have we can add it in the verify part of this script

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These is no such API available. Checking if libtorch_xpu.so => libmkl_sycl_*.so is an alternative method.

@CuiYifeng CuiYifeng force-pushed the yifeng/rename_mkl_env branch from 88de14e to 78f38e6 Compare May 20, 2025 05:43
@CuiYifeng CuiYifeng requested a review from dvrogozh May 20, 2025 05:44
@CuiYifeng CuiYifeng added this pull request to the merge queue May 22, 2025
Merged via the queue into main with commit 3475b8e May 22, 2025
18 of 21 checks passed
@CuiYifeng CuiYifeng deleted the yifeng/rename_mkl_env branch May 22, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants