-
Notifications
You must be signed in to change notification settings - Fork 147
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
decreases memory usage by 6 times for MERRA2 #774
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I am wondering: In physics/aerinterp.F90
, lines 231/232-288/290 and lines 437/438-494/498 are essentially the same. Should this be put in a private subroutine in the same file and called from the two places?
@@ -256,15 +351,20 @@ END SUBROUTINE setindxaer | |||
!********************************************************************** | |||
!********************************************************************** | |||
! | |||
SUBROUTINE aerinterpol(me,master,nthrds,npts,IDATE,FHOUR,jindx1,jindx2, & | |||
SUBROUTINE aerinterpol( me,master,nthrds,npts,IDATE,FHOUR,iflip, jindx1,jindx2, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUBROUTINE aerinterpol( me,master,nthrds,npts,IDATE,FHOUR,iflip, jindx1,jindx2, & | |
SUBROUTINE aerinterpol(me,master,nthrds,npts,IDATE,FHOUR,iflip, jindx1,jindx2, & |
Dom, It is only repeated two/three times. I am afraid using a new
subroutine might slow the code down by defining a new set of variables in
the new subroutine. A few arrays occupy large memory, relocating and
delocating might take time.
…On Tue, Nov 16, 2021 at 9:02 AM Dom Heinzeller ***@***.***> wrote:
***@***.**** requested changes on this pull request.
This looks good. I am wondering: In physics/aerinterp.F90, lines
231/232-288/290 and lines 437/438-494/498 are essentially the same. Should
this be put in a private subroutine in the same file and called from the
two places?
------------------------------
In physics/aerinterp.F90
<#774 (comment)>:
> @@ -256,15 +351,20 @@ END SUBROUTINE setindxaer
!**********************************************************************
!**********************************************************************
!
- SUBROUTINE aerinterpol(me,master,nthrds,npts,IDATE,FHOUR,jindx1,jindx2, &
+ SUBROUTINE aerinterpol( me,master,nthrds,npts,IDATE,FHOUR,iflip, jindx1,jindx2, &
⬇️ Suggested change
- SUBROUTINE aerinterpol( me,master,nthrds,npts,IDATE,FHOUR,iflip, jindx1,jindx2, &
+ SUBROUTINE aerinterpol(me,master,nthrds,npts,IDATE,FHOUR,iflip, jindx1,jindx2, &
------------------------------
In physics/aerinterp.F90
<#774 (comment)>:
> @@ -310,6 +415,88 @@ SUBROUTINE aerinterpol(me,master,nthrds,npts,IDATE,FHOUR,jindx1,jindx2, &
endif
enddo
n1 = n2 - 1
+ if (n2 > 12) n2 = n2 -12
+! need to read a new month
+ if (n1.ne.n1sv) then
+ if (me == master) write(*,*)"read in a new month MERRA2", n2
⬇️ Suggested change
- if (me == master) write(*,*)"read in a new month MERRA2", n2
+#ifdef DEBUG
+ if (me == master) write(*,'(a,i3)') "Reading in MERRA2 data for month", n2
+#endif
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#774 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIOGI742FQ237TFCPATUMJP6NANCNFSM5HVS2IFA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
The compiler will easily take care of this, maybe even inline the new subroutine. Code duplication is bad and should be avoided if possible. |
Dom, I am trying to put the code in a subroutine now. Let see how it goes,
…On Tue, Nov 16, 2021 at 10:23 AM Dom Heinzeller ***@***.***> wrote:
Dom, It is only repeated two/three times. I am afraid using a new
subroutine might slow the code down by defining a new set of variables in
the new subroutine. A few arrays occupy large memory, relocating and
delocating might take time.
… <#m_-8902789229747407544_>
The compiler will easily take care of this, maybe even inline the new
subroutine. Code duplication is bad and should be avoided if possible.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#774 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIOBD4LPMGR6V6UORKLUMJZPVANCNFSM5HVS2IFA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Dom,
I have just completed the changes by putting the repeated lines in
subroutine read_netfaer. The code passed regression tests and the timing is
about the same. The changes have been pushed in. Great idea!
Anning
On Tue, Nov 16, 2021 at 11:24 AM Anning Cheng - NOAA Affiliate <
***@***.***> wrote:
… Dom, I am trying to put the code in a subroutine now. Let see how it
goes,
On Tue, Nov 16, 2021 at 10:23 AM Dom Heinzeller ***@***.***>
wrote:
> Dom, It is only repeated two/three times. I am afraid using a new
> subroutine might slow the code down by defining a new set of variables in
> the new subroutine. A few arrays occupy large memory, relocating and
> delocating might take time.
> … <#m_-776242511803897711_m_-8902789229747407544_>
>
> The compiler will easily take care of this, maybe even inline the new
> subroutine. Code duplication is bad and should be avoided if possible.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#774 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ALQPMIOBD4LPMGR6V6UORKLUMJZPVANCNFSM5HVS2IFA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great, thanks a lot. I had made one suggestion for change in aerinterp.F90
aroind line 286, please let me know if you can't see it.
dom, just made the change and pushed in, which should save some time.
…On Tue, Nov 16, 2021 at 2:52 PM Dom Heinzeller ***@***.***> wrote:
***@***.**** commented on this pull request.
Looking great, thanks a lot. I had made one suggestion for change in
aerinterp.F90 aroind line 286, please let me know if you can't see it.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#774 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQPMIIVUBOK7HZJR2FMWU3UMKZAPANCNFSM5HVS2IFA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. Will approve after the regression testing is completed. We need to make sure that this PR andd the parent PRs for fv3atm and ufs-weather-model are all up to date with the latest development, and that the submodule pointers are all correct.
decrease of the memory usage for MERRA2 aerosol climatology by reading 2 month of data instead of 12 months. When the model is running, the new data is read in the interpolation subroutine. The performance of the model has also improved by 6%. as shown in the two figures below
fix #774
fix # ufs-community/ufs-weather-model#903
w
.