Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix deprecated oneMKL call #1718

Merged
merged 9 commits into from
Nov 19, 2024
Merged

Fix deprecated oneMKL call #1718

merged 9 commits into from
Nov 19, 2024

Conversation

MarcelKoch
Copy link
Member

@MarcelKoch MarcelKoch commented Oct 29, 2024 •
edited
Loading

This PR uses the new overload of release_matrix_handle and set_csr_data instead of the deprecated one, see https://www.intel.com/content/www/us/en/docs/onemkl/developer-reference-dpcpp/2025-0/oneapi-mkl-sparse-release-matrix-handle.html and https://www.intel.com/content/www/us/en/docs/onemkl/developer-reference-dpcpp/2025-0/oneapi-mkl-sparse-set-csr-data.html
Also, the member functions for shuffle in subgroup and is_host() are removed in the oneapi 2025.0.0
Use the group algorithm and remove is_host() in this pr.
The group algorithm should still be supported from 2023.1

Fixes #1717

Todo:

  • wait until intel CI is running again.
  • check for other removed oneAPI calls.

@MarcelKoch MarcelKoch added this to the Ginkgo 1.9.0 milestone Oct 29, 2024
@MarcelKoch MarcelKoch self-assigned this Oct 29, 2024
@ginkgo-bot ginkgo-bot added reg:benchmarking This is related to benchmarking. type:matrix-format This is related to the Matrix formats mod:dpcpp This is related to the DPC++ module. labels Oct 29, 2024
@MarcelKoch MarcelKoch added the 1:ST:do-not-merge Please do not merge PR this yet. label Oct 29, 2024
@yhmtsai
Copy link
Member

yhmtsai commented Oct 29, 2024

ah, you are also working on that.
you need to change the set_csr_data, too.

@yhmtsai
Copy link
Member

yhmtsai commented Oct 29, 2024

I am preparing the docker image such that we can check the compilation at least

@MarcelKoch
Copy link
Member Author

@yhmtsai feel free to push if you already have a fix for set_csr_data.

@yhmtsai yhmtsai force-pushed the fix-deprecated-mkl-call branch 2 times, most recently from 02cd5de to 736b9b3 Compare October 29, 2024 18:17
@MarcelKoch MarcelKoch force-pushed the fix-deprecated-mkl-call branch from 736b9b3 to 1d2cb4e Compare November 13, 2024 10:11
@MarcelKoch MarcelKoch changed the base branch from develop to intel-ci-cluster November 13, 2024 10:12
@MarcelKoch
Copy link
Member Author

format!

@MarcelKoch MarcelKoch requested a review from pratikvn November 13, 2024 14:11
pratikvn
pratikvn reviewed Nov 13, 2024
View reviewed changes
Copy link
Member

@pratikvn pratikvn left a comment

Choose a reason for hiding this comment

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

Which version of SYCL do we support now ? Everything starting from 2023.1 ? I think we should have additional jobs for one 2024 and also one 2025 version.

@MarcelKoch
Copy link
Member Author

@pratikvn We currently test 2023.1 and 2024.1 (this is the oneapi:latest image). For me that is good enough, but we can also try to add 2025 if there is enough time before the release.

@MarcelKoch MarcelKoch force-pushed the fix-deprecated-mkl-call branch from fd02afb to 59f7a91 Compare November 14, 2024 12:57
@upsj upsj force-pushed the intel-ci-cluster branch 2 times, most recently from 123a9e3 to 7e6c81c Compare November 15, 2024 08:55
Base automatically changed from intel-ci-cluster to develop November 16, 2024 21:05
@MarcelKoch MarcelKoch force-pushed the fix-deprecated-mkl-call branch from 59f7a91 to ee6af20 Compare November 18, 2024 12:30
@MarcelKoch
[sycl] use newer MKL target if available
7e6391d
@MarcelKoch
[sycl] use non-deprecated MKL call only after v2024
b0f81df 
Somehow the 'correct' version of the MKL calls lead to a segfault if version < 2024, even though they already deprecated the old call in v2023...
@MarcelKoch MarcelKoch force-pushed the fix-deprecated-mkl-call branch from ee6af20 to b0f81df Compare November 18, 2024 12:32
upsj
upsj approved these changes Nov 18, 2024
View reviewed changes
Copy link
Member

@upsj upsj left a comment

Choose a reason for hiding this comment

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

LGTM!

yhmtsai
yhmtsai reviewed Nov 18, 2024
View reviewed changes
Copy link
Member

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

because I did partial changes in this code, I do not give approval from my side.
I only have one question for MKL_VERSION dispatch, others look good to me

benchmark/utils/dpcpp_linops.dp.cpp
Comment on lines +56 to +58
#if INTEL_MKL_VERSION >= 20240000
*exec->get_queue(),
#endif
Copy link
Member

Choose a reason for hiding this comment

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

from the documentation, the old one should deprecated in the 2023.0.
Doesn't it provide the new one even in 2023.1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but I think the new one doesn't work in 2023.1. At least running on the PVC@FTP it always segfaulted.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for clarifying it

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
5.1% Duplication on New Code

See analysis details on SonarQube Cloud

@codecov Codecov
Copy link

codecov bot commented Nov 18, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.10%. Comparing base (c051384) to head (b0f81df).
Report is 10 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop    #1718   +/-   ##
========================================
  Coverage    90.10%   90.10%           
========================================
  Files          782      782           
  Lines        63474    63475    +1     
========================================
+ Hits         57192    57193    +1     
  Misses        6282     6282

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@MarcelKoch MarcelKoch added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:do-not-merge Please do not merge PR this yet. labels Nov 19, 2024
@MarcelKoch MarcelKoch merged commit 8304552 into develop Nov 19, 2024
11 of 14 checks passed
@MarcelKoch MarcelKoch deleted the fix-deprecated-mkl-call branch November 19, 2024 07:39
MarcelKoch added a commit to MarcelKoch/ginkgo that referenced this pull request Dec 2, 2024
@MarcelKoch
Merge Fix deprecated oneMKL calls
bf83936 
This PR uses the new overload of release_matrix_handle and set_csr_data instead of the deprecated one, see https://www.intel.com/content/www/us/en/docs/onemkl/developer-reference-dpcpp/2025-0/oneapi-mkl-sparse-release-matrix-handle.html and https://www.intel.com/content/www/us/en/docs/onemkl/developer-reference-dpcpp/2025-0/oneapi-mkl-sparse-set-csr-data.html. Also, the member functions for shuffle in subgroup and is_host() are removed in the oneapi 2025.0.0, and replaced with the group algorithm (which is also supported by 2023.1).

Related PR: ginkgo-project#1718
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pratikvn pratikvn pratikvn left review comments

@yhmtsai yhmtsai yhmtsai left review comments

@upsj upsj upsj approved these changes

Assignees

@MarcelKoch MarcelKoch

Labels
1:ST:ready-to-merge This PR is ready to merge. 1:ST:run-full-test mod:dpcpp This is related to the DPC++ module. reg:benchmarking This is related to benchmarking. type:matrix-format This is related to the Matrix formats
Projects
None yet
Milestone
Ginkgo 1.9.0
Development

Successfully merging this pull request may close these issues.

Update for MKL 2025
5 participants
@MarcelKoch @yhmtsai @upsj @pratikvn @ginkgo-bot