Skip to content

Conversation

@basava70
Copy link
Collaborator

PR to test OpenACC compilation and running in main branch.

@JanStreffing
Copy link
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
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

@JanStreffing JanStreffing added the enhancement New feature or request label Feb 25, 2025
@basava70
Copy link
Collaborator Author

Is everything good ? @JanStreffing

@JanStreffing
Copy link
Collaborator

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

@basava70
Copy link
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
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
Copy link
Collaborator Author

basava70 commented Feb 26, 2025

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
Collaborator

add as job_gpu_levante

@basava70
Copy link
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
Copy link
Member

pgierz commented Feb 26, 2025

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

Copy link
Collaborator

@JanStreffing JanStreffing left a 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!

Copy link
Collaborator

@suvarchal suvarchal left a 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
Copy link
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.

#ifdef ENABLE_OPENACC
!$ACC end parallel loop
#endif
!$OMP PARALLEL DO
Copy link
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
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
Copy link
Collaborator

suvarchal commented Jun 19, 2025

@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
Copy link
Collaborator

mandresm commented Jun 19, 2025

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
Copy link
Collaborator Author

basava70 commented Jun 27, 2025

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
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
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
Collaborator Author

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

@basava70
Copy link
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
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.

6 participants