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

Fix OpenACC rountine issue for subgrid wetting and drying #6472

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

sbrus89
Copy link
Contributor

@sbrus89 sbrus89 commented Jun 13, 2024

This PR moves a subgrid subroutine call out of a OpenACC parallel region to fix the compile problems noted in #6470. Since subgrid wetting and drying is strictly a MPAS-Ocean standalone feature, I believe it should be fine for this code to remain CPU-only.

This PR also fixes a couple issues in mpas_ocn_vmix.F:

  • An OpenACC bug related to the use of gang vector collapse(3) on a double nested loop with variable inner loop bounds.
  • A missing !$omp parallel region in a calculation for the config_use_gotm option.

[BFB] -- mpas-ocean standalone

Copy link
Contributor

@philipwjones philipwjones left a comment

Choose a reason for hiding this comment

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

I'm about to head out on vacation so can't give this a proper review - but this looks ok except that you probably want to update the device copies of any array that is changed in the loop and make sure the input args have been similarly updated on host. I will go ahead and approve assuming someone else can test and check whether all arrays have been appropriately synchronized.

@mark-petersen mark-petersen self-requested a review June 20, 2024 15:20
Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

I compiled this with gnu (debug and optimized) on chicoma and intel on chrysalis and tested with the nightly test suite. This works fine. I tried to run buttermilk bay and the parabolic bowl on chicoma, but it was failing because it didn't find the mesh file, unrelated to this PR. Anyway, the code changes are clear and I am comfortable with this change.

@xylar
Copy link
Contributor

xylar commented Jun 21, 2024

Approved by code inspection and based on @sbrus89's testing. I think we should just keep the vmix fixes here because otherwise we'll have too many PRs.

@sbrus89
Copy link
Contributor Author

sbrus89 commented Jun 21, 2024

I've added the host/device updates as @philipwjones suggested and tested one more time. All stages of SMS_Ld1.T62_oEC60to30v3.CMPASO-NYF.pm-gpu_nvidiagpu pass with the combination of this PR and #6471.

@jonbob jonbob added the BFB PR leaves answers BFB label Jun 24, 2024
jonbob added a commit that referenced this pull request Jun 24, 2024
Fix OpenACC rountine issue for subgrid wetting and drying

This PR moves a subgrid subroutine call out of a OpenACC parallel region
to fix the compile problems noted in #6470. Since subgrid wetting and
drying is strictly a MPAS-Ocean standalone feature, it should be fine
for this code to remain CPU-only.

This PR also fixes a couple issues in mpas_ocn_vmix.F:
* An OpenACC bug related to the use of gang vector collapse(3) on a
  double nested loop with variable inner loop bounds.
* A missing !$omp parallel region in a calculation for the
  config_use_gotm option.

[BFB] -- mpas-ocean standalone only
@jonbob
Copy link
Contributor

jonbob commented Jun 24, 2024

Passes:

  • ERS_Ld5.T62_oQU120.CMPASO-NYF.chrysalis_intel
  • SMS_D_Ld1.TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF.chrysalis_intel.mpaso-jra_1958

merged to next

@jonbob jonbob merged commit a08e5fe into E3SM-Project:master Jun 25, 2024
13 checks passed
@jonbob
Copy link
Contributor

jonbob commented Jun 25, 2024

merged to master

xylar added a commit to xylar/compass that referenced this pull request Jul 3, 2024
This merge updates the E3SM-Project submodule from [31f771c](https://github.com/E3SM-Project/E3SM/tree/31f771c) to [c7d7998](https://github.com/E3SM-Project/E3SM/tree/c7d7998).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6472
- [ ]  (ocn) E3SM-Project/E3SM#6471
- [ ]  (ocn) E3SM-Project/E3SM#6481
xylar added a commit to xylar/compass that referenced this pull request Jul 4, 2024
This merge updates the E3SM-Project submodule from [31f771c](https://github.com/E3SM-Project/E3SM/tree/31f771c) to [c7d7998](https://github.com/E3SM-Project/E3SM/tree/c7d7998).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6472
- [ ]  (ocn) E3SM-Project/E3SM#6471
- [ ]  (ocn) E3SM-Project/E3SM#6481
xylar added a commit to xylar/compass that referenced this pull request Jul 4, 2024
This merge updates the E3SM-Project submodule from [31f771c](https://github.com/E3SM-Project/E3SM/tree/31f771c) to [c7d7998](https://github.com/E3SM-Project/E3SM/tree/c7d7998).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6472
- [ ]  (ocn) E3SM-Project/E3SM#6471
- [ ]  (ocn) E3SM-Project/E3SM#6481
xylar added a commit to xylar/compass that referenced this pull request Jul 4, 2024
This merge updates the E3SM-Project submodule from [31f771c](https://github.com/E3SM-Project/E3SM/tree/31f771c) to [c7d7998](https://github.com/E3SM-Project/E3SM/tree/c7d7998).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6472
- [ ]  (ocn) E3SM-Project/E3SM#6471
- [ ]  (ocn) E3SM-Project/E3SM#6481
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB bug fix PR mpas-ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants