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.

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

Production de openacc test #589

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Production de openacc test #589

wants to merge 11 commits into from

Conversation

basava70
Copy link
Collaborator

This branch has the working OpenACC code. I had to comment out
set(CMAKE_EXE_LINKER_FLAGS) line in src/CMakeLists.txt, among other things. But adding this line seems to break the code. Review it before merging.
Also this code is taking 25 seconds for 1 day. I think this is in accordance what @suvarchal got in LUMI.

Copy link
Collaborator

@mandresm mandresm left a comment

Choose a reason for hiding this comment

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

Hi @basava70,

Thanks for your PR. I have left you some reviews already. Can you please address the comments and suggestions?

CMakeLists.txt Outdated Show resolved Hide resolved
env.sh Outdated Show resolved Hide resolved
env/levante.dkrz.de/shell.nvhpc Outdated Show resolved Hide resolved
Comment on lines -61 to +63

set(NV_GPU_ARCH "cc80" CACHE STRING "GPU arch for nvfortran compiler (cc35,cc50,cc60,cc70,cc80,...)")
option(DISABLE_OPENACC_ATOMICS "disable kernels using atomic statement for reproducible results" ON)
set(GPU_COMPUTE_CAPABILITY "cc80" CACHE STRING "GPU arch for nvfortran compiler (cc35,cc50,cc60,cc70,cc80,...)")
set(GPU_FLAGS "cuda12.2,${GPU_COMPUTE_CAPABILITY}" CACHE STRING "GPU arch for nvfortran compiler (cc35,cc50,cc60,cc70,cc80,...)")
Copy link
Collaborator

@mandresm mandresm May 21, 2024

Choose a reason for hiding this comment

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

@suvarchal, this was different from a previous version that was working on Levante. @basava70 replaced this line to have a variable that contains not only cc80 but cuda12.2,cc80 that is then passed to the -gpu flag during compilation. Does this change make sense to you?

src/CMakeLists.txt Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
set(CMAKE_EXE_LINKER_FLAGS "-acc -ta=tesla:${NV_GPU_ARCH}")
message("Taking ENABLE_OPENACC = ON")
target_compile_options(${PROJECT_NAME} PRIVATE -acc -O2 -gpu=${GPU_FLAGS} -Minfo=accel)
# set(CMAKE_EXE_LINKER_FLAGS "-acc -gpu=${GPU_FLAGS}")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@suvarchal @dsidoren @mandresm
I had to comment this particular line for the OpenACC code to work. I am not sure why. If anyone can let me know why, it will be great. But the code is running with this change.

@basava70 basava70 requested a review from mandresm May 21, 2024 11:27
@mandresm
Copy link
Collaborator

Hi @basava70, thanks for the changes, I have no further comments

@mandresm
Copy link
Collaborator

Why was this closed?

@JanStreffing
Copy link
Collaborator

JanStreffing commented Aug 15, 2024

I was cleanin up branches that I contributed to and that were already merged to main. By accident I cleaned up production_DE in the process. This then closed the PR.

@JanStreffing
Copy link
Collaborator

I have restored the branch, but consider rebasing to main, as production_DE has already been merged. Developments merged into production_DE will be lost unless you make a second merge request of production_DE into main.

@JanStreffing JanStreffing reopened this Aug 15, 2024
@basava70 basava70 changed the base branch from production_DE to main August 26, 2024 13:11
@JanStreffing JanStreffing added this to the FESOM 2.6.1 milestone Aug 26, 2024
@JanStreffing JanStreffing self-assigned this Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants