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

Addressed Execution issues in atmosphere/develop-openacc branch and included various code optimizations #1243

Open
wants to merge 10 commits into
base: atmosphere/develop-openacc
Choose a base branch
from

Conversation

Pranay-Reddy-Kommera
Copy link

This PR addresses the execution issues while simulating the 'atmosphere/develop-openacc' branch.

In addition, different OpenACC kernel optimizations are also applied to the OpenACC code in both the Dynamics and Physics routines.

@mgduda mgduda requested review from mgduda and gdicker1 November 1, 2024 20:40
@mgduda mgduda added Atmosphere OpenACC Work related to OpenACC acceleration of code labels Nov 1, 2024
Copy link
Collaborator

@gdicker1 gdicker1 left a comment

Choose a reason for hiding this comment

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

@Pranay-Reddy-Kommera, thanks for these changes. I have two things that I think should be addressed.

Once the error with mpas_atmphys_lsm_noahinit.F is solved, this should be good to go!

@@ -123,17 +123,17 @@ pgi:
"CC_SERIAL = pgcc" \
"CXX_SERIAL = pgc++" \
"FFLAGS_PROMOTION = -r8" \
"FFLAGS_OPT = -O4 -byteswapio -Mfree" \
"FFLAGS_OPT = -O4 -gopt -byteswapio -Mfree -Mnosave -Mrecursive -Mstack_arrays -DMPAS_NVTX_RANGES" \
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the develop-openacc branch, could you duplicate this pgi entry and name it nvhpc? Just to align more with what current users may expect to use in their make command.

Choose a reason for hiding this comment

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

@gdicker1 I understand you suggestion and I agree having it as nvhpc would be appropriate. I will push the change as soon as possible.

@@ -434,7 +435,7 @@ subroutine soil_veg_gen_parm(dminfo,mminlu,mminsl)
endif
if(sltype.eq.mminsl) then
do lc = 1, slcats
read(read_unit,*) iindex,bb(lc),drysmc(lc),f11(lc),maxsmc(lc),refsmc(lc),satpsi(lc), &
read(read_unit,*) iindex,bb(lc),drysmc(lc),hc(lc), f11(lc),maxsmc(lc),refsmc(lc),satpsi(lc), &
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't run this code as is. Whenever I try, I get this error message (on stdout/stderr):

  My role is             3
  Role leader is             0
 FIO-F-231/list-directed read/unit=3/error on data conversion.
  File name = 'SOILPARM.TBL',    formatted, sequential access   record = 4
  In source file mpas_atmphys_lsm_noahinit.F, at line number 438
 deg0066.hsn.de.hpc.ucar.edu: rank 0 exited with code 127

I get the same error when I use my unmodified copy of MPAS-A_benchmark_60km_v7.0 and when I override the physics files with the ones that are in the top-level of MPAS-Model after building this branch. Maybe a TWC-specific change?

Choose a reason for hiding this comment

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

The error might be because the SOILPARM.TBL might not have the 'hc' column in it. It looks like we are using different files. Are there multiple versions of SOILPARM.TBL files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there multiple versions of SOILPARM.TBL files?

Yes. These files come from MPAS-Dev/MPAS-Data and are cloned into "src/core_atmosphere/physics/physics_wrf/files" based on the version of the model. See the checkout_data_files.sh script which is used in the "src/core_atmosphere/physics/Makefile".

I can't find a version of mpas_atmphys_lsm_noahinit.F in this repo (going through tags and openacc branches) that has a read statement including hc(lc). This leads me to believe there is no version of SOILPARAM.TBL that a reasonable user has access to or would get during the build process that allows them to run these changes.

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

Successfully merging this pull request may close these issues.

3 participants