-
Notifications
You must be signed in to change notification settings - Fork 67
Openacc test main #693
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
Openacc test main #693
Conversation
|
GCC compile fails with: See: https://github.com/FESOM/fesom2/pull/693/checks#step:5:207 |
I am going through them now. Thanks |
|
Is everything good ? @JanStreffing |
|
should I add the job_hpc_levante to the code as well ? I am using that file to run the code. |
How does it differ from job_levante? |
First difference would be it would use gpu's, so there are |
|
add as job_gpu_levante |
Should I also add a comment on how to build ? |
|
Hi @basava70, I would put one working runscript and one working compile script into the examples folder so everything is documented. I have not built FESOM outside of the esm-tools framework in a while, but if I remember correctly they also have example build scripts... |
|
I have merged with the latest commits from main. The code is finally compiling with OpenACC and running with both nvhpc and intel compilers on GPU and CPU respectively. |
JanStreffing
left a comment
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.
Looks to be ready!
suvarchal
left a comment
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.
could you take out the commits from #697 https://github.com/FESOM/fesom2/pull/697/commits because it is not ready and test again, remaining are optional if you promise to fix them in next PR about to come.
| #endif | ||
|
|
||
| !$ACC PARALLEL LOOP GANG VECTOR DEFAULT(PRESENT) VECTOR_LENGTH(acc_vl) | ||
| !$OMP DO |
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.
ifndef ENABLE_OPENACC is missing here and few omp blocks below, not sure openmp and openacc will work with this file but if it is too much we can defer to next PR you intend to do.
| #ifdef ENABLE_OPENACC | ||
| !$ACC end parallel loop | ||
| #endif | ||
| !$OMP PARALLEL DO |
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.
not guarded openmp meaning we can´t use openmp with openacc for other parts. can defer to next PR if you want to.
| !$OMP PARALLEL DEFAULT(SHARED) PRIVATE(n, nz) | ||
| !$ACC PARALLEL LOOP GANG VECTOR DEFAULT(PRESENT) DEFAULT(PRESENT) VECTOR_LENGTH(acc_vl) | ||
| !$ACC PARALLEL LOOP GANG VECTOR DEFAULT(PRESENT) VECTOR_LENGTH(acc_vl) | ||
| !$OMP DO |
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.
not guarded openmp meaning we can´t use openmp with openacc for other parts. can defer to next PR if you want to.
|
@basava70 apart from testing for gpu, cpu on levante; can you also enable openmp without openacc and test - in case code is broken as there is lot of ifdefs nested with openmp and openacc. |
|
Thanks a lot for your review @suvarchal! |
(cherry picked from commit 237434c)
f92e83e to
5b7447f
Compare
|
Hi all,
Please do review and merge. credits: I used chatgpt to create the table in markdown. |
|
@suvarchal, I think this is ready for another round of review. |
|
@basava70 have you tested with ./configure.sh --clean levante.intel -DENABLE_OPENMP=ON and in job script export OMP_NUM_THREADS=whatever_slurm_cpus_per_task just a check before if it can be merged, i mean with intel or gnu not nvhpc because you changed some definition naming slightly in cmakelists and in code. we can defer both ENABLE_OPENACC and OPENMP (which i think will not work now) to your next |
|
@suvarchal No, I didn't. I will update you in a couple of hours. |
I could compile it with this option. cpu time = 92.67 seconds |
|
Time to merge? |
PR to test OpenACC compilation and running in main branch.