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

Add OpenACC module to core_test #1175

Merged
merged 5 commits into from
May 31, 2024

Conversation

gdicker1
Copy link
Collaborator

This PR adds a new module to the test core that will be used to test OpenACC functionality and any OpenACC-specific framework routines that are added to MPAS. The only test added in this PR is based on patterns from the MPAS-A dycore. If the test core is compiled without OPENACC=true, the OpenACC tests are skipped.

@gdicker1
Copy link
Collaborator Author

Also includes changes to LDFLAGS that are in #1174.

@mgduda mgduda self-requested a review May 30, 2024 19:54
@mgduda mgduda added the OpenACC Work related to OpenACC acceleration of code label May 30, 2024
@gdicker1 gdicker1 changed the title Add module to test OpenACC to core_test Add OpenACC module to core_test May 30, 2024
@mgduda
Copy link
Contributor

mgduda commented May 30, 2024

Also includes changes to LDFLAGS that are in #1174.

With PR #1174 merged, I think it would be safe to rebase this PR.

src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
src/core_test/Registry.xml Outdated Show resolved Hide resolved
src/core_test/mpas_test_core.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_core.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
src/core_test/mpas_test_openacc.F Show resolved Hide resolved
@gdicker1 gdicker1 force-pushed the framework/openacc_test branch from b5d2a99 to 690909e Compare May 31, 2024 02:12
@gdicker1
Copy link
Collaborator Author

With PR #1174 merged, I think it would be safe to rebase this PR.

Addressed by force push from b5d2a99 to 690909e

@gdicker1 gdicker1 force-pushed the framework/openacc_test branch 2 times, most recently from 10fb55b to cdc2ac0 Compare May 31, 2024 03:32
@gdicker1 gdicker1 requested a review from mgduda May 31, 2024 03:34
Copy link
Contributor

@mgduda mgduda left a comment

Choose a reason for hiding this comment

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

I think this looks generally good. Just two minor comments to address.

src/core_test/mpas_test_openacc.F Outdated Show resolved Hide resolved
@gdicker1 gdicker1 force-pushed the framework/openacc_test branch from cdc2ac0 to 054078d Compare May 31, 2024 16:56
@gdicker1
Copy link
Collaborator Author

These comments should be addressed by the push from cdc2ac0 to 054078d

I removed the diff_arrs_1d routine, removed the use mpas_dmpar ... from openacc_test_rep_arrs, and initialized diff to 0.0 in openacc_test_rep_arrs

@gdicker1 gdicker1 requested a review from mgduda May 31, 2024 16:59
Copy link
Contributor

@mgduda mgduda left a comment

Choose a reason for hiding this comment

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

The final state of the PR branch looks good to me. But, some commits (e.g., a2a82b2) don't compile with the GNU 13.2.0 compilers:

mpas_test_openacc.F:343:10:

  343 |         #ifdef MPAS_OPENACC
      |          1
Error: Invalid character in name at (1)

If it's easier, feel free to just squash all of the commits into a single commit. Otherwise, if you'd like to preserve all commits, I think you may need to go back to the first commit on the branch to resolve the pre-processing directive issue.

gdicker1 added 5 commits May 31, 2024 16:18
Right now this just contains a stub routine, but this gets some of the
"plumbing work" done.
Guard the call to mpas_test_openacc routine with ifdef MPAS_OPENACC so
the routines are only run if the test_core is built with OpenACC
enabled.
This test carries out some calculations on the CPU and the GPU based on
patterns in the dynamics of core_atmosphere. If the difference between
the CPU results and the GPU results is 0, the test succeeds.
@gdicker1 gdicker1 force-pushed the framework/openacc_test branch from 054078d to b276568 Compare May 31, 2024 22:20
@gdicker1
Copy link
Collaborator Author

Thank you for pointing that out, I neglected to fix the macro spacing in the first commit in the sequence. I checked with gnu on Derecho, the push from 054078d to b276568 addresses this.

@gdicker1 gdicker1 requested a review from mgduda May 31, 2024 22:26
Copy link
Contributor

@mgduda mgduda left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenACC Work related to OpenACC acceleration of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants