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

New Velocity Potential and Stream Function Calculations #1072

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

KarinaAsmar-NOAA
Copy link
Contributor

This PR adds the CPC-requested streamfunction and velocity potential at 200mb to SFS. It is meant to resolve [Issue #902 ] and update on the runtime issues from the previous PR #951 . Job scripts used for testing are in WCOSS2: /lfs/h2/emc/vpppg/noscrub/karina.asmar/vpot_strm/UPP (submit_run_gfsv16_wcoss2.sh and submit_run_sfs_wcoss2.sh).

@KarinaAsmar-NOAA
Copy link
Contributor Author

@WenMeng-NOAA We have updated the computation for streamfunction and velocity potential for compatibility with UPP parallelization. The subroutine now uses a poisson solver with a convergence tester to reduce runtimes. Comparison of spectral and numerical results is here

@GeorgeVandenberghe-NOAA
Copy link
Contributor

The poisson solver works but is substantially slower than the gather --> stptranf --> scatter operation to solve the equation spectrally. The gather is much cheaper than it sounds because it is TWO fields, (U and V) at one level, not the several hundred such fields that comprise state plus derivative variables. A gather of two fields takes about 0.06 seconds on hera at GFS (high) resolution and it hard to even measure at CFS resolution. The relaxations take about 10 seconds on hera for chi and psi together and less than a second for the spectral solver. Spectral solver takes 27 seconds for the GFS (high) resolution case while the poisson solver takes 161 seconds. Spectral solver takes less than a second for the CFS case while the poisson solver takes about three seconds.

@@ -81,6 +84,9 @@ SUBROUTINE COLLECT_LOC ( A, B )
deallocate(buff)
deallocate(rbufs)

tb=mpi_wtime()
Copy link
Collaborator

Choose a reason for hiding this comment

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

@KarinaAsmar-NOAA Clean up the debugging code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GeorgeVandenberghe-NOAA Would you please clean up the debugging part of COLLECT_LOC.f? Let me know when done and I'll push it to this branch.

@@ -104,6 +110,8 @@ SUBROUTINE COLLECT_ALL ( A, B )
real, dimension(im,jm), intent(out) :: b
integer ierr,n
real, allocatable :: rbufs(:)
real*8 tb,ta
Copy link
Collaborator

Choose a reason for hiding this comment

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

@KarinaAsmar-NOAA Clean up the debugging code in this routine.

@WenMeng-NOAA WenMeng-NOAA changed the title Add STRM and VPOT at 200mb to SFS New Velocity Potential and Stream Function Calculations Dec 4, 2024
@WenMeng-NOAA WenMeng-NOAA added SFSV1 enhancement New feature or request labels Dec 4, 2024
IF(J>1 .and. J<JM) THEN
pval=chi(i,j)
CHI(I,J) = 0.25*( PTMP(I-1,J)+PTMP(I+1,J)+PTMP(I,J-1)+PTMP(I,J+1) &
- DIV(I,J)/(wrk2(i,j)*wrk1(i,j)*wrk3(i,j)*wrk1(i,j)*COSL(i,j)*4.))
Copy link
Contributor

@DusanJovic-NOAA DusanJovic-NOAA Dec 4, 2024

Choose a reason for hiding this comment

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

I think entire sub-expression:

- DIV(I,J)/(wrk2(i,j)*wrk1(i,j)*wrk3(i,j)*wrk1(i,j)*COSL(i,j)*4.

can be computed outside the 'DO jj=1,300000' loop. A compiler might see this and optimize the code, but it might not, so I think it's worth trying'

Copy link
Contributor

Choose a reason for hiding this comment

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

@DusanJovic-NOAA @KarinaAsmar-NOAA
Good idea! Maybe can reduce a few seconds. Worth trying. Also the ABSV-F term above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WenMeng-NOAA Please let us know if the new changes with suggestions from @DusanJovic-NOAA @JesseMeng-NOAA reduce the runtimes

Comment on lines 4672 to 4673
! npass = npass2
! if (j > jm-jtem+1 .or. j < jtem) npass = npass3
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove if not needed

@@ -1816,6 +1819,101 @@ SUBROUTINE MDL2P(iostatusD3D)
ENDIF
ENDIF
!
!*** CHIPSI
!
IF (IGET(1021) > 0 .or. IGET(1022) > 0) THEN
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JesseMeng-NOAA @KarinaAsmar-NOAA This new scheme will only applied in global applications. Please change as:

IF ( (IGET(1021) > 0 .or. IGET(1022) > 0). and. MODELNAME == 'GFS' ) THEN

@@ -4497,6 +4500,483 @@ SUBROUTINE CALSLR_UUTAH(SLR)
END SUBROUTINE CALSLR_UUTAH
!
!-------------------------------------------------------------------------------------
!
!> Computes streamfunction and velocity potential from absolute vorticity
Copy link
Collaborator

Choose a reason for hiding this comment

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

@KarinaAsmar-NOAA Since this is a new subroutine incorporated in UPP, you might reformat the fortran code provided by @JesseMeng-NOAA and @GeorgeVandenberghe-NOAA readable and consistent as:

  • all lowercase
  • indentation for loops, conditionals and blocks

@GeorgeVandenberghe-NOAA
Copy link
Contributor

GeorgeVandenberghe-NOAA commented Dec 6, 2024 via email

@KarinaAsmar-NOAA
Copy link
Contributor Author

You should change "GWVX RELAX TIME " to something like "RELAX TIME" or "POISSON SOLUTION TIME".

On Fri, Dec 6, 2024 at 6:55 PM KarinaAsmar-NOAA @.> wrote: @.* commented on this pull request. ------------------------------ In sorc/ncep_post.fd/UPP_PHYSICS.f <#1072 (comment)>: > + ENDDO + + CHI=0. + tb=mpi_wtime() + if (me .eq. 5) print 109,' GWVX RELAX TIME ',tb-ta + 109 format(a,f20.10,i10) + DO jj=1,300000 + call exch(chi(ista_2l:iend_2u,jsta_2l:jend_2u)) + PTMP=CHI + err=0 + DO J=JSTA,JEND + DO I=ISTA,IEND + IF(J>1 .and. J<JM) THEN + pval=chi(i,j) + CHI(I,J) = 0.25*( PTMP(I-1,J)+PTMP(I+1,J)+PTMP(I,J-1)+PTMP(I,J+1) & + - DIV(I,J)/(wrk2(i,j)wrk1(i,j)wrk3(i,j)wrk1(i,j)COSL(i,j)4.)) @WenMeng-NOAA https://github.com/WenMeng-NOAA Please let us know if the new changes with suggestions from @DusanJovic-NOAA https://github.com/DusanJovic-NOAA @JesseMeng-NOAA https://github.com/JesseMeng-NOAA reduce the runtimes — Reply to this email directly, view it on GitHub <#1072 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANDS4FUGVSAMGB7UCBLPHST2EHXIHAVCNFSM6AAAAABQK7FXP6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOBVGY2DGNRZGA . You are receiving this because you were mentioned.Message ID: @.>
-- George W Vandenberghe Lynker Technologies at * NOAA/NWS/NCEP/EMC 5830 University Research Ct., Rm. 2141 College Park, MD 20740 @.
301-683-3769(work) 3017751547(cell)

@GeorgeVandenberghe-NOAA Sure I can change that. Are the rest of the 'GWVX' necessary or can they be removed?

@WenMeng-NOAA WenMeng-NOAA added Ready for Review This PR is ready for code review. and removed On hold labels Dec 6, 2024
@WenMeng-NOAA
Copy link
Collaborator

@KarinaAsmar-NOAA Your recent commits have increase the runtime to 2381 S.

@KarinaAsmar-NOAA
Copy link
Contributor Author

@KarinaAsmar-NOAA Your recent commits have increase the runtime to 2381 S.

@WenMeng-NOAA Which commit needs to be removed? 808bc84 or b67fb2c?

@KarinaAsmar-NOAA
Copy link
Contributor Author

@KarinaAsmar-NOAA Your recent commits have increase the runtime to 2381 S.

@WenMeng-NOAA Which commit needs to be removed? 808bc84 or b67fb2c?

@WenMeng-NOAA It looks like the issue was removing the if lvls block (see comment). I restored it and tested with SFS data. Please let me know if runtimes are acceptable now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Ready for Review This PR is ready for code review. SFSV1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt the calculations of velocity potential and streamfunction in UPP
5 participants