Skip to content

Openacc test main#693

Merged
suvarchal merged 15 commits into
mainfrom
openacc_test_main
Jun 30, 2025
Merged

Openacc test main#693
suvarchal merged 15 commits into
mainfrom
openacc_test_main

Conversation

@basava70

Copy link
Copy Markdown
Collaborator

PR to test OpenACC compilation and running in main branch.

@JanStreffing

Copy link
Copy Markdown
Collaborator

GCC compile fails with:

[ 53%] Building Fortran object src/CMakeFiles/fesom.dir/oce_tracer_mod.F90.o
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:30:0:

   30 | #endif
      | 
Error: #endif without #if
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:41:0:

   41 | #endif
      | 
Error: #endif without #if
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:28:6:

   28 |     #ifdef ENABLE_OPENACC
      |      1
Error: Invalid character in name at (1)
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:39:6:

   39 |     #ifdef ENABLE_OPENACC
      |      1
Error: Invalid character in name at (1)
make[2]: *** [src/CMakeFiles/fesom.dir/build.make:1492: src/CMakeFiles/fesom.dir/oce_tracer_mod.F90.o] Error 1
make[2]: *** Waiting for unfinished jobs....

See: https://github.com/FESOM/fesom2/pull/693/checks#step:5:207

@JanStreffing
JanStreffing self-requested a review February 25, 2025 11:29
@basava70

Copy link
Copy Markdown
Collaborator Author

GCC compile fails with:

[ 53%] Building Fortran object src/CMakeFiles/fesom.dir/oce_tracer_mod.F90.o
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:30:0:

   30 | #endif
      | 
Error: #endif without #if
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:41:0:

   41 | #endif
      | 
Error: #endif without #if
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:28:6:

   28 |     #ifdef ENABLE_OPENACC
      |      1
Error: Invalid character in name at (1)
/__w/fesom2/fesom2/src/oce_tracer_mod.F90:39:6:

   39 |     #ifdef ENABLE_OPENACC
      |      1
Error: Invalid character in name at (1)
make[2]: *** [src/CMakeFiles/fesom.dir/build.make:1492: src/CMakeFiles/fesom.dir/oce_tracer_mod.F90.o] Error 1
make[2]: *** Waiting for unfinished jobs....

See: https://github.com/FESOM/fesom2/pull/693/checks#step:5:207

I am going through them now. Thanks

Comment thread env/levante.dkrz.de/shell.intel
Comment thread env/levante.dkrz.de/shell.nvhpc
Comment thread src/oce_tracer_mod.F90 Outdated
@JanStreffing JanStreffing added the enhancement New feature or request label Feb 25, 2025
@basava70

Copy link
Copy Markdown
Collaborator Author

Is everything good ? @JanStreffing

@JanStreffing

Copy link
Copy Markdown
Collaborator

Tests pass. I'm happy. @mandresm and @dsidoren, may comment on the ACC changes.

@basava70

Copy link
Copy Markdown
Collaborator Author

should I add the job_hpc_levante to the code as well ? I am using that file to run the code.

@JanStreffing

Copy link
Copy Markdown
Collaborator

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?

@basava70

basava70 commented Feb 26, 2025

Copy link
Copy Markdown
Collaborator Author

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
#SBATCH --gpus=
#SBATCH --gpus_per_task
and few others
should I post the file as a comment ?
there are also few other export commands, that was needed on levante gpu for ICON to work, else segmentation fault. This file was created in consultation with @suvarchal a few months ago. May you dont need these exports anymore but I am not sure.

@JanStreffing

Copy link
Copy Markdown
Collaborator

add as job_gpu_levante

@basava70

Copy link
Copy Markdown
Collaborator Author

add as job_gpu_levante

Should I also add a comment on how to build ?
./configure.sh levante.nvhpc -DENABLE_OPENACC=ON -DDISABLE_OPENACC_ATOMICS=OFF
currently I use this to build. You think it is necessary to add this a comment in the job_gpu_levante file ?

@pgierz

pgierz commented Feb 26, 2025

Copy link
Copy Markdown
Member

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...

@basava70
basava70 requested a review from suvarchal June 19, 2025 13:10
@basava70

Copy link
Copy Markdown
Collaborator Author

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.
Please review and merge at your latest convenience or let me know your comments and what changes would you like to see.

@JanStreffing JanStreffing left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks to be ready!

Comment thread env/levante.dkrz.de/shell.nvhpc Outdated

@suvarchal suvarchal left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread cmake/AddPreprocessTarget.cmake Outdated
Comment thread src/gen_modules_partitioning.F90
#endif

!$ACC PARALLEL LOOP GANG VECTOR DEFAULT(PRESENT) VECTOR_LENGTH(acc_vl)
!$OMP DO

Copy link
Copy Markdown
Collaborator

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.

Comment thread src/oce_tracer_mod.F90
#ifdef ENABLE_OPENACC
!$ACC end parallel loop
#endif
!$OMP PARALLEL DO

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Collaborator

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.

@suvarchal

suvarchal commented Jun 19, 2025

Copy link
Copy Markdown
Collaborator

@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. ./configure.sh --clean levante.intel -DENABLE_OPENMP=ON and in job script export OMP_NUM_THREADS=whatever_slurm_cpus_per_task you use before running fesom.x.

@mandresm

mandresm commented Jun 19, 2025

Copy link
Copy Markdown
Collaborator

Thanks a lot for your review @suvarchal!

@basava70
basava70 force-pushed the openacc_test_main branch from f92e83e to 5b7447f Compare June 19, 2025 18:10
@basava70

basava70 commented Jun 27, 2025

Copy link
Copy Markdown
Collaborator Author

Hi all,
I have finished the work I need to do in this branch.
I completed the benchmark comparing runtimes between cpu and gpu versions.

Run Length Run Length Unit Steps per Day CPU (s) GPU (s)
1 d 32 1.806 125.2253
1 m 32 55.408 4160.025
1 y 32 787.221 > 26400.0
2 y 32 1287.22 NA

Please do review and merge.
Thanks

credits: I used chatgpt to create the table in markdown.

@JanStreffing

Copy link
Copy Markdown
Collaborator

@suvarchal, I think this is ready for another round of review.

@JanStreffing
JanStreffing requested a review from suvarchal June 27, 2025 09:40
@suvarchal

Copy link
Copy Markdown
Collaborator

@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
PR.

@basava70

Copy link
Copy Markdown
Collaborator Author

@suvarchal No, I didn't. I will update you in a couple of hours.

@basava70

Copy link
Copy Markdown
Collaborator Author

@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 PR.

I could compile it with this option.
OMP_NUM_THREADS=2
--ntasks=256
--ntasks-per-node=64
--cpus-per-task=2

cpu time = 92.67 seconds
run_length = 1
run_length_unit = 'm'

@JanStreffing

Copy link
Copy Markdown
Collaborator

Time to merge?

@suvarchal
suvarchal merged commit c3fdac7 into main Jun 30, 2025
7 checks passed
@basava70 basava70 mentioned this pull request Jun 30, 2025
5 tasks
@JanStreffing
JanStreffing deleted the openacc_test_main branch June 30, 2025 08:08
@basava70 basava70 mentioned this pull request Jul 1, 2025
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants