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 MKL linking on pm-cpu_intel #5993

Merged
merged 1 commit into from
Oct 13, 2023
Merged

Fix MKL linking on pm-cpu_intel #5993

merged 1 commit into from
Oct 13, 2023

Conversation

jgfouca
Copy link
Member

@jgfouca jgfouca commented Oct 13, 2023

Need to tell cmake to use Intel vendor for blas.

[BFB]

@jgfouca
Fix MKL linking on pm-cpu_intel
a063972 
Need to tell cmake to use Intel vendor for blas.

[BFB]
@jgfouca jgfouca added BFB PR leaves answers BFB CMake build system labels Oct 13, 2023
@jgfouca jgfouca requested review from ndkeen and dqwu October 13, 2023 16:21
@jgfouca jgfouca self-assigned this Oct 13, 2023
@github-actions
Copy link

PR Preview Action v1.4.4
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-5993/
on branch gh-pages at 2023-10-13 16:23 UTC

@jgfouca
Copy link
Member Author

jgfouca commented Oct 13, 2023 •
edited
Loading

I'm not 100% sure that the BLA_VENDOR=Intel10_64_dyn is what we want, but it will get pm-cpu working again and I will follow up with Kitware.

@ndkeen
Copy link
Contributor

ndkeen commented Oct 13, 2023

I'm communicating with Jim on slack as well as trying to ask others at NERSC. I'd be OK with this change for now to get tests passing (assuming we do get same results/performance), and we can revisit later. ?

dqwu
dqwu approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@dqwu dqwu left a comment

Choose a reason for hiding this comment

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

Manually tested this fix on pm-cpu with intel compiler.

jgfouca added a commit that referenced this pull request Oct 13, 2023
@jgfouca
Merge branch 'jgfouca/fix_pm_intel_blas' into next (PR #5993)
d0406ad 
Fix MKL linking on pm-cpu_intel

Need to tell cmake to use Intel vendor for blas.

[BFB]
@jgfouca jgfouca merged commit 495ba1d into master Oct 13, 2023
2 checks passed
@jgfouca jgfouca deleted the jgfouca/fix_pm_intel_blas branch October 13, 2023 18:22
@ndkeen
Copy link
Contributor

ndkeen commented Oct 13, 2023

I see you already merged this to master.

I did some quick benchmarks on 8 node of pm-cpu, with and without openmp.
The performance looks the same to me. I don't have a good feel for the percent time spent in MKL here.

ne30pg2_EC30to60E2r2.F2010, 5 days

Oct4th master
8 nodes, 128x1, 8.61 SYPD
8 nodes,  64x2, 8.81 SYPD

jgfouca_fix_pm_intel_blas
8 nodes, 128x1, 8.77 SYPD
8 nodes,  64x2, 8.79 SYPD

@jgfouca jgfouca mentioned this pull request Oct 19, 2023
Open
49 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dqwu dqwu dqwu approved these changes

@ndkeen ndkeen Awaiting requested review from ndkeen

Assignees

@jgfouca jgfouca

Labels
BFB PR leaves answers BFB CMake build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jgfouca @ndkeen @dqwu