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

Enable UFS-GOCART integration #664

Merged
merged 24 commits into from
Jun 30, 2021

Conversation

rmontuoro
Copy link
Contributor

@rmontuoro rmontuoro commented May 24, 2021

This PR introduces changes required to integrate the NASA GOCART model into the UFS (#663) along with a few bug fixes.

Added features:

  • Exported 10-meter wind components
  • Exported instantaneous 3D non-convective liquid and ice precipitation fluxes from GFDL cloud microphysics

Fixes:

  • Properly handle error flag in PBL generic scheme when aerosol diffusion is enabled. This bug prevented aerosols from being diffused within the PBL
  • Initialize work arrays in SAMF deep and shallow convection schemes
  • Initialize wet deposition work array and ensure only proper elements of cloud base mass flux array are used in SAMF aerosol deep and shallow convective transport schemes
  • Export correct values for the upward sensible heat flux over the ocean when cplflx is .true. and coupling with aerosol component is enabled.

Regression test log files for the following platforms available at https://github.com/rmontuoro/ufs-weather-model/tree/feature/gocart:

  • hera.intel
  • orion.intel
  • wcoss_dell_p3

Copy link
Collaborator

@climbfuji climbfuji 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, just one tiny change to revert.

physics/GFS_surface_generic.F90 Outdated Show resolved Hide resolved
Copy link
Collaborator

@climbfuji climbfuji 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, will approve when the regression testing is done. Thanks!

@rmontuoro rmontuoro requested a review from climbfuji June 3, 2021 19:36
@@ -91,7 +92,7 @@ subroutine samfdeepcnv_aerosols(im, ix, km, itc, ntc, ntr, delt,

do k = 1, km
do i = 1, im
xmbp(i,k) = g * xmb(i) / delp(i,k)
if (cnvflg(i)) xmbp(i,k) = g * xmb(i) / delp(i,k)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a question: I understand that it is better for memory access to have the nested loop in k,i order, but does inserting the IF statement within the inner loop change the computational performance in practice such that the loop order should be reversed? I.e. Does the benefit of ordering the loop k,i overcome the expense of running IF i*k times? (Also repeated below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cost (CPU cycles) associated with memory access is usually much larger than the cost of branching inside loops due to optimized branch prediction available in modern processors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the explanation!

Copy link
Collaborator

@grantfirl grantfirl 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. One comment about inserting an i-dependent IF statement within a nested k,i loop.

@climbfuji
Copy link
Collaborator

@SMoorthi-emc please take a look at the RAS changes: 7f6e45e - thanks!

@SMoorthi-emc
Copy link
Collaborator

SMoorthi-emc commented Jun 30, 2021 via email

endif
if (itc > 0 .and. ntc > 0) then
if (ntr >= itc + ntc - 3) then
fscav_(itc:ntc-1) = fscav
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @SMoorthi-emc - this should be

My previous email included a typo. Apologies

@climbfuji
Copy link
Collaborator

Dom, I assume "itc" is the starting address and "ntc" is the ending address of chemistry variables, to be subjected to scavenging. So, itc is >2 and ntc could be <=ntr and the dimension of "fscav) would be (ntc-itc+1). If so, don't you think it should be "fscav_(itc:ntc) = fscav"? For example, say there is only "one" chemistry variable. Then itc=ntc right? Moorthi

Of course, Moorthi. Thanks for catching it, I updated the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants