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 7 commits into
base: production_DE
Choose a base branch
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

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.

None yet

2 participants