Reimplementation of extensions to OpenACC region in time integration split routine #4954
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philipwjones
approved these changes
May 20, 2022
| !$acc enter data copyin(layerThicknessCur) | ||
| !$acc enter data copyin(layerThicknessTend) | ||
| !$acc enter data copyin(normalBaroclinicVelocityCur) | ||
| if (varinp_compute_active_tracer_budgets) then |
There was a problem hiding this comment.
@brobey I did a test merge into master, and when I compiled I get this error:
mpas_ocn_time_integration_split.f90:1705:52:
1705 | if (varinp_compute_active_tracer_budgets) then
| 1
Error: Symbol ‘varinp_compute_active_tracer_budgets’ at (1) has no IMPLICIT type
make[3]: *** [mpas_ocn_time_integration_split.o] Error 1
Were all these varinp variables removed in another PR? It might be easiest to `
git rebase origin/master
and then recompile and fix these.
There was a problem hiding this comment.
Yes, the other merge went in before this, so it just needs to be rebased.
There was a problem hiding this comment.
I rebased to the E3SM project master and changed the varinp variables to config variables. The branch now compiles and passes on Darwin with OpenACC.
|
If the other PR went in, it would then be config_compute_active_tracer_budgets instead of varinp_compute_active_tracer_budgets.
--
Bob Robey
XCP-2, Computational Physics
505/412-8114 or 505/665-9052
Pronouns: he/him/his
From: Mark Petersen ***@***.***>
Reply-To: E3SM-Project/E3SM ***@***.***>
Date: Wednesday, May 25, 2022 at 5:43 PM
To: E3SM-Project/E3SM ***@***.***>
Cc: Robert Robey ***@***.***>, Mention ***@***.***>
Subject: [EXTERNAL] Re: [E3SM-Project/E3SM] Reimplementation of extensions to OpenACC region in time integration split routine (PR #4954)
@mark-petersen commented on this pull request.
________________________________
In components/mpas-ocean/src/mode_forward/mpas_ocn_time_integration_split.F<https://urldefense.com/v3/__https:/github.com/E3SM-Project/E3SM/pull/4954*discussion_r882202684__;Iw!!Bt8fGhp8LhKGRg!VfROMY0ENnPIUKHNffAU6MZzFfvnxPvIpoFQRXDVeTlVvMND2z0HnIcpCWROcaE$>:
+ if ( associated(frazilSurfacePressure) ) then
+ !$acc enter data copyin(frazilSurfacePressure)
+ endif
+ if (landIcePressureOn) then
+ !$acc enter data copyin(landIcePressure)
+ !$acc enter data copyin(landIceDraft)
+ endif
+ if (config_use_freq_filtered_thickness) then
+ !$acc enter data copyin(highFreqThicknessNew)
+ !$acc enter data copyin(highFreqThicknessCur)
+ endif
+ elseif (splitExplicitStep == numTSIterations) then
+ !$acc enter data copyin(layerThicknessCur)
+ !$acc enter data copyin(layerThicknessTend)
+ !$acc enter data copyin(normalBaroclinicVelocityCur)
+ if (varinp_compute_active_tracer_budgets) then
@brobey<https://urldefense.com/v3/__https:/github.com/brobey__;!!Bt8fGhp8LhKGRg!VfROMY0ENnPIUKHNffAU6MZzFfvnxPvIpoFQRXDVeTlVvMND2z0HnIcpypg2Pf0$> I did a test merge into master, and when I compiled I get this error:
mpas_ocn_time_integration_split.f90:1705:52:
1705 | if (varinp_compute_active_tracer_budgets) then
| 1
Error: Symbol ‘varinp_compute_active_tracer_budgets’ at (1) has no IMPLICIT type
make[3]: *** [mpas_ocn_time_integration_split.o] Error 1
Were all these varinp variables removed in another PR? It might be easiest to `
git rebase origin/master
and then recompile and fix these.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/E3SM-Project/E3SM/pull/4954*pullrequestreview-985585264__;Iw!!Bt8fGhp8LhKGRg!VfROMY0ENnPIUKHNffAU6MZzFfvnxPvIpoFQRXDVeTlVvMND2z0HnIcp3FC961I$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AAJ6ALFLQWXAI5PJRCIQALLVL23JPANCNFSM5WFNTXRQ__;!!Bt8fGhp8LhKGRg!VfROMY0ENnPIUKHNffAU6MZzFfvnxPvIpoFQRXDVeTlVvMND2z0HnIcp8GeD9oQ$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
mark-petersen
force-pushed
the
brobey/mpas-ocean/openacc-newextendsplit
branch
from
99c4fe9
to
d2f2da0
Compare
|
@brobey I rebased on e3sm-project/master, and that version passes stand-alone nightly suite with gnu debug and optimized, and matches bfb with master. Since I rebased the code, please take a look through and run your standard test, to make sure it came through as expected. |
|
I had rebased with e3sm-project/master, so the additional rebase only had a few small changes. I reviewed these changes by inspection and there should be no impact. |
|
Tested on Darwin with OpenACC and passes bfb with rebase. |
|
Also double-checked on summit GPUs w/ OpenACC after rebase and still bfb there. Looks good. |
mark-petersen
approved these changes
Jun 7, 2022
jonbob
added a commit
that referenced
this pull request
Jun 7, 2022
…4954) Reimplementation of extensions to OpenACC region in time integration split routine Reimplementation of extensions to OpenACC region to encompass the entire if blocks for the split explicit step: * Extending the OpenACC region to encompass all of the large iteration if block * Extending OpenACC region outside the if splitExplicitStep blocks * Fixing loop in diagnostic solve to run on GPU These changes are separate and independent from the addition of the OpenACC top of the split routine changes. [BFB]
|
Test merge passes:
merged to next |
|
merged to master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reimplementation of extensions to OpenACC region to encompass the entire if blocks for the split explicit step. The prior pull request #4853 was corrupted when it was rebased to the changes in the head version. The reimplementation is done with a more recent head version to minimize difficulties with the merges of the changes
These changes are separate and independent from the addition of the OpenACC top of the split routine changes.
QU240 test passes on Darwin with NVHPC (ver 21.11), GCC (ver 9), and Intel (ver 19) builds. Test suite on Cori Haswell with the Intel compiler passes on the tests that pass with the head version (3 fail with the head version). I still don't have access to Chrysalis or Summit to test on those platforms. Changes are bit-for-bit identical.