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 ncol/pcols mismatch on interpolating aerosol mass fields to pressure levels #6260

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

wlin7
Copy link
Contributor

@wlin7 wlin7 commented Feb 26, 2024

The input dimension size specified in call vertinterp to interpolate aerosol masses to
pressure levels can be larger than the actual mass array (pcols >= ncol). This can cause
unintended values, either from different levels or random values outside the memory
space associated with the array being used for interpolation, resulting in NaN values for
some configuration or on some machines. Even though not resulting in NaN values for a run,
such mismatch is also expected to cause the interpolated arrays being corrupted.

Fixes #6259.

[BFB] Differences only for diagnostic fields such as mass_bc_850 in eam.h0, if they are saved for a test

@wlin7 wlin7 requested a review from mahf708 February 26, 2024 02:59
@wlin7 wlin7 self-assigned this Feb 26, 2024
Copy link

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6260/
on branch gh-pages at 2024-02-26 03:02 UTC

@mahf708
Copy link
Contributor

mahf708 commented Feb 26, 2024

Since you filed #6259, I was trying to remember how I came up with the calls PR #5862. I simply copied the logic from elsewhere and used it for the aerosol fields. So, this likely is more widespread issue than the aerosol fields. For example, the current calls for the interpolated-to-pressure aerosol fields look like:

call vertinterp(ncol, pcols, pver, pmid,  85000._r8, mass_3d_tmp, mass_at_pressure)

Similar calls can be found in different places. Some examples below.

components/eam/src/physics/cam/gw_drag.F90

call vertinterp(ncol, pcols, pver, state%pmid, 5000._r8, dummyx, dummmy_p_surf)

components/eam/src/physics/rrtmg/radiation.F90

call vertinterp(ncol, pcols, pverp, state%pint, 20000._r8, fns, fsn200)

components/eam/src/physics/cam/cam_diagnostics.F90

call vertinterp(ncol, pcols, pver, state%pmid, 80000._r8, state%t, p_surf)

components/eam/src/physics/cam/zm_conv_intr.F90

call vertinterp(ncol, pcols, pver, state%pmid, 60000._r8, state%u,du600)

Should we fix these other calls in this PR or are they sufficiently different? I know we have a pressing interest in these specific fields for the campaigns coming up, so we could simply punt on the other issues until later.

@wlin7
Copy link
Contributor Author

wlin7 commented Feb 26, 2024

Thanks for the quick review, @mahf708 . Those other interpolations are ok. Take the following as an example
call vertinterp(ncol, pcols, pver, state%pmid, 80000._r8, state%t, p_surf)

The 2nd argument specify the column dimension sizes for state%pmid and stat%t, and they are both declared with pcols. The actual calculation loops over the 1st dimension, i.e., ncol in this call.

For the aerosol mass arrays, such as mass_bc and the tmp array mass_3d_tmp, they are declared as (ncol,pver), need to use ncol for the 2nd argument and the explicit scoping for pmid to ensure the match. Dummy argument ncold in subroutine vertinterp dictate all these.

@wlin7 wlin7 added this to the v3.0.0-rc milestone Feb 26, 2024
@mahf708
Copy link
Contributor

mahf708 commented Feb 26, 2024

Ah, I see! Thanks! I was obviously lazy to figure out all the nuances here, and I will do better next time (or I should manage to learn enough f90) before I edit any substantial code 😸

@wlin7
Copy link
Contributor Author

wlin7 commented Feb 26, 2024

Ah, I see! Thanks! I was obviously lazy to figure out all the nuances here, and I will do better next time (or I should manage to learn enough f90) before I edit any substantial code 😸

That is fine. We all reuse codes often by copying/pasting. Everybody could miss something like this sometimes.

wlin7 added a commit that referenced this pull request Feb 28, 2024
Fix ncol/pcols mismatch on interpolating aerosol mass fields to pressure levels

The input dimension size specified in call vertinterp to interpolate aerosol masses to
pressure levels can be larger than the actual mass array (pcols >= ncol). This can cause
unintended values, either from different levels or random values outside the memory
space associated with the array being used for interpolation, resulting in NaN values for
some configuration or on some machines. Even though not resulting in NaN values for a run,
such mismatch is also expected to cause the interpolated arrays being corrupted.

Fixes #6259.

[BFB] Differences only for diagnostic fields such as mass_bc_850 in eam.h0, if they are saved for a test
@wlin7
Copy link
Contributor Author

wlin7 commented Feb 28, 2024

Merged to next.

@wlin7 wlin7 merged commit cad2d2a into master Feb 28, 2024
10 checks passed
@wlin7 wlin7 deleted the wlin/atm/fix_aerosol_mass_vertinterp branch February 28, 2024 20:44
@rljacob rljacob modified the milestones: v3.0.0-rc1, v3.0.0-rc2 Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ne120pg2 run on pm-cpu failed on writing aerosol mass at specified levels to eam.h0
3 participants