Optimize GSI#527
Conversation
…nal FV3 DA (FV3LAMDA)
|
As per the 4/10/2023 email sent to the GSI subversion email list, PRs must be merged within 6 weeks of creation. PRs not merged within the 6 week window will be returned to the originating developer (see GSI Wiki). This PR was opened before this announcement . The due date for this PR is set to 5/22/2023, six weeks after 4/10/2023. |
|
Changes based on Cathy's comments completed and pushed to branch. Successful compile. Will run tests soon. |
|
I checked out the branch and the develop to run single-cycle tests ( The code changes look good to me. |
|
Regression tests run. Look OK. Also ensured that my trunk is up to date with GSI trunk. No differences. I think it is ready to go. |
CatherineThomas-NOAA
left a comment
CatherineThomas-NOAA
left a comment
There was a problem hiding this comment.
Thanks for your patience @jderber-NOAA. There's a lot of work in this PR and the timing results are good. I'll be sure to use that extra time up immediately in GFSv17 :)
Code looks good to me. Approved.
|
Reminder: This review will be closed and returned to the developer if not merged before 5/23/2023 |
|
<!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
-->Russ, I thought both Cathy and Emily approved this change. What do I/we need to do to get it in? John Sent from Mail for Windows From: RussTreadon-NOAASent: Monday, May 8, 2023 9:32 AMTo: NOAA-EMC/GSICc: jderber-NOAA; MentionSubject: Re: [NOAA-EMC/GSI] Optimize GSI (PR #527) Reminder: This review will be closed and returned to the developer if not merged before 5/23/2023—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I do not see an approval from Emily. |
|
Given approvals from @CatherineThomas-NOAA and @emilyhcliu this PR can now be processed for merger into |
|
Merge request submitted to GSI Handling Review Team. |
|
Majority of handling review team concur with merger. Merge will be completed shortly. |
| ! Declare derived constants | ||
| integer(i_kind):: huge_i_kind | ||
| integer(i_kind), parameter :: max_varname_length=64 | ||
| integer(i_kind), parameter :: max_varname_length=20 |
There was a problem hiding this comment.
@jderber-NOAA In my regional run, I just the max_varname-length=20 is not enough to contain names like" fv3SAR01_ens_mem001-fv3_dynvars" .
^
character(len=max_varname_length) :: filenamein2
V
Would you consider changing this to a bigger value like original one? Or I should use a different size (another parameter) for my use?
Thanks.
There was a problem hiding this comment.
Yes. It is in the list of changes I am currently working on.
There was a problem hiding this comment.
John, Thanks a lot!
|
<!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
code
{mso-style-priority:99;
font-family:"Courier New";}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:"Courier New";}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
-->Note this change has already been put into trunk. A modification will require another PR. Sent from Mail for Windows From: TingLei-NOAASent: Friday, May 12, 2023 7:14 PMTo: NOAA-EMC/GSICc: jderber-NOAA; MentionSubject: Re: [NOAA-EMC/GSI] Optimize GSI (PR ***@***.*** commented on this pull request.In src/gsi/constants.f90:> @@ -90,7 +90,7 @@ module constants ! Declare derived constants integer(i_kind):: huge_i_kind- integer(i_kind), parameter :: max_varname_length=64+ integer(i_kind), parameter :: ***@***.*** In my regional run, I just the max_varname-length=20 is not enough to contain names like" fv3SAR01_ens_mem001-fv3_dynvars" .^character(len=max_varname_length) :: filenamein2VWould you consider changing this to a bigger value like original one? Or I should use a different size (another parameter) for my use?Thanks. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
6 participants
This update is intended to optimize the GSI to make it run faster in operations.
The changes are primarily optimization changes (reducing data movement, adding threading, reducing memory) but include a few minor changes when small bugs were present. For example there was an inconsistency between the intsst and stpsst routines for certain observations. The difference was very small when sst observations were used.
In addition, there was a change to eliminate the observation output files when no observations of that type were used. Thus, if no lightning data was used, the observation output file was not created. This results in the production of fewer files in each run and eliminates the cost of creating the files.
A more complete description of the changes can be found in issue #525.
The code is found in jderber-NOAA/GSI/optimize.
In all of the regression tests and in my testing the differences with between the control and the test were on the order of round-off differences or slightly larger. All testing was done on Hera since this is the only machine I have access to.
Fixes #525