add fstats customized for screen-level Q and T with "land only" option #246

saraqzhang · 2022-02-23T15:14:49Z

a standalone set of code, script, and rc file are added to GEOS_Util/post/ for forecast stats customized for 2-meter moisture and temperature verification. It is tested in the cases of self-verification and ecmwf-verification, and with "land only" option.

modified: CMakeLists.txt new file: g5fcst_statsNx.pl new file: statsNx.F90 new file: statsNx.rc

modified: GEOS_Util/post/CMakeLists.txt

saraqzhang · 2022-02-23T16:07:46Z

@lltakacs @gmao-jstassi Larry and Joe, the modifications/additions ( for screen level variables and land only option) are not put into fstats.F90, g5fcst_stats.pl and stats.rc, instead in the customized fstatsNx.F90, g5fcst_statsNx.pl and statsNx.rc. Comparison of *stats vs. *statsNx files will show the modifications. could you review them and give comments and suggestions. We have tested and used this addition for fcst verifications in land-atmos coupled DAS experiments.

saraqzhang · 2022-03-10T01:24:22Z

Larry and Joe reviewed the draft PR , and sent via email comments and suggestions:

for fstatsNx.F90
" (1) The variable “landonly” is only explicitly initialized if “-land” is present. You should probably set a default value explicitly to .false.
(2) There are several comments and print-outs that are different from the ones in the Develop version of GIT. I think you might just have started from an older version. Never-the-less, it would be best to merge your updates once again to what is in Develop." -Larry

for g5fcst_statsNx.pl
"I did a xxdiff between the scripts, and I saw that a couple dozen lines have been added or modified. I didn't see anything that looked like a problem. Ideally, you should get someone else to run the code to see it does what they think it should do.
If I get a chance later, after you get your script into the repository, I will take a look and try to merge the two scripts." - Joe

gmao-rreichle · 2022-03-22T02:12:16Z

@saraqzhang, re. you last commit:

merge statsNx.F90 to stats.F90

If stats.F90 can now do everything that stats.F90 and statsNx.F90 were doing when you first created the PR, then we should "git remove" statsNx.F90 from the branch before merging the PR.

modified: GEOS_Util/post/g5fcst_stats.pl

gmao-rreichle · 2022-03-31T13:29:28Z

GEOS_Util/post/g5fcst_stats.pl

+# name - g5fcst_stats.pl 
+# with options of land only and 2d Nx with screen level variables only 
+# purpose - script to submit jobs to calculate screen level forecast statistics


The help text here seems wrong. Should it be something like:

# name - g5fcst_stats.pl # purpose - script to submit jobs to calculate forecast statistics # options: land-only stats and stats for screen-level variables (2d Nx)

gmao-rreichle · 2022-03-31T13:33:06Z

GEOS_Util/post/g5fcst_stats.pl

+    #print ("nxonly option: $nxonly\n");
+    #print ("landonly option: $nxonly\n");


These lines should probably be deleted. (The commented-out "landonly" print statement isn't right anyway.)

saraqzhang · 2022-04-04T16:49:30Z

the modification/addition for new options "land only" and "Nx only" are merged into the stats script and code. The "customized" version of the codes are removed.

gmao-rreichle · 2022-04-04T17:08:50Z

GEOS_Util/post/CMakeLists.txt

 ecbuild_add_executable (TARGET stats.x SOURCES stats.F90)
+ecbuild_add_executable (TARGET statsNx.x SOURCES statsNx.F90)


Don't you also need to remove "statsNx" here and below?

CMakelists is just updated . we still keep statsNx.rc .

sdrabenh · 2022-05-02T16:40:40Z

@saraqzhang would you mind updating the changelog for this PR so all CI tests will pass? Thanks

gmao-rreichle · 2022-05-02T16:43:59Z

GEOS_Util/post/g5fcst_stats.pl

-# name - g5fcst_stats.pl
-# purpose - script to submit jobs to calculate forecast statistics
+# name - g5fcst_stats.pl 
+# purpose - script to submit jobs to calculate screen level forecast statistics


The edit in this comment doesn't seem to be correct. Isn't this the merged script that does the regular atm stats and now also screen-level?

gmao-rreichle · 2022-05-02T16:50:23Z

GEOS_Util/post/stats.F90

+                          Td = q(i,j)-273.15 ! to C   
+                          pp = sp(i,j)/100.  ! to mb 
+                          ee= 6.112*exp((17.67*Td)/(Td + 243.5)) 
+                          q(i,j) = (0.622 * ee)/(pp - (0.378 * ee))


Could some or all of this conversion be done using MAPL functions? If so, we should probably use them. If there aren't MAPL functions, it may still be possible to use MAPL constants instead of hard-wired numbers for some of the numbers.

gmao-rreichle · 2022-05-02T16:51:04Z

GEOS_Util/post/statsNx.rc

+##sqz  NOTE : this file has a copy 
+##           in /discover/nobackup/qzhang/git_check/GEOSadas/install/etc/
+##           the run actually use that one instead of the one here. 


Delete these three comment lines before merging?

…MAO_Shared into feature/saraqzhang/fstats2mQT update sandbox feature/saraqzhang/fstats2mQT

saraqzhang · 2022-05-03T01:48:06Z

this code is compiled/built with MAPL v2.8.0 used in adas, where MAPL Const only have MAPL_TICE, not MAPL_CELSIUS_TO_KELVIN.

gmao-rreichle · 2022-05-03T01:56:08Z

this code is compiled/built with MAPL v2.8.0

Ouch, that's old. Oh well. I guess for the purpose of computing fstats it doesn't really matter whether 273.15 or 273.16 is used.

sdrabenh · 2022-05-03T13:26:04Z

@gmao-rreichle @saraqzhang do you feel this PR is resolved and ready for merging?

gmao-rreichle · 2022-05-03T14:20:26Z

@gmao-rreichle @saraqzhang do you feel this PR is resolved and ready for merging?

I'm happy with the changes.

mathomp4

Approved for CMake

saraqzhang added 2 commits February 22, 2022 16:51

addtional fstats customized for screen-level Q and T

b50c120

modified: CMakeLists.txt new file: g5fcst_statsNx.pl new file: statsNx.F90 new file: statsNx.rc

add install g5fcst_statsNx.pl to BIN

9476b6c

modified: GEOS_Util/post/CMakeLists.txt

saraqzhang added enhancement New feature or request Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs labels Feb 23, 2022

saraqzhang requested review from gmao-jstassi and gmao-rreichle February 23, 2022 15:14

saraqzhang self-assigned this Feb 23, 2022

merge statsNx.F90 to stats.F90

cc6d96c

merge modifications for screen level variable stats

9ca8240

modified: GEOS_Util/post/g5fcst_stats.pl

gmao-rreichle reviewed Mar 31, 2022

View reviewed changes

saraqzhang added 2 commits April 4, 2022 12:22

modification of comments

2add96a

remove statsNx script and code

e26c21f

remove statsNx from installation in CMakeLists

3268d4e

gmao-rreichle reviewed Apr 4, 2022

View reviewed changes

saraqzhang and others added 2 commits April 19, 2022 14:04

add exit & error message if landmask file not found (landonly option)

14794e6

Merge branch 'main' into feature/saraqzhang/fstats2mQT

bd3b303

saraqzhang marked this pull request as ready for review April 19, 2022 18:13

saraqzhang requested review from a team as code owners April 19, 2022 18:13

saraqzhang added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. and removed Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs labels Apr 19, 2022

Merge branch 'main' into feature/saraqzhang/fstats2mQT

0fcd85e

gmao-rreichle reviewed May 2, 2022

View reviewed changes

saraqzhang added 3 commits May 2, 2022 20:51

add MAPL_Const in stats.F90, and comment line edits

93ba5e2

Merge branch 'feature/saraqzhang/fstats2mQT' of github.com:GEOS-ESM/G…

262419d

…MAO_Shared into feature/saraqzhang/fstats2mQT update sandbox feature/saraqzhang/fstats2mQT

update CHANGELOG.md

7b1f1e0

saraqzhang requested a review from a team as a code owner May 3, 2022 01:23

sdrabenh approved these changes May 3, 2022

View reviewed changes

gmao-rreichle approved these changes May 3, 2022

View reviewed changes

mathomp4 approved these changes May 3, 2022

View reviewed changes

sdrabenh merged commit fcec98b into main May 3, 2022

sdrabenh deleted the feature/saraqzhang/fstats2mQT branch May 3, 2022 14:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add fstats customized for screen-level Q and T with "land only" option #246

add fstats customized for screen-level Q and T with "land only" option #246

saraqzhang commented Feb 23, 2022

saraqzhang commented Feb 23, 2022

saraqzhang commented Mar 10, 2022

gmao-rreichle commented Mar 22, 2022

gmao-rreichle Mar 31, 2022

gmao-rreichle Mar 31, 2022

saraqzhang commented Apr 4, 2022

gmao-rreichle Apr 4, 2022

saraqzhang Apr 4, 2022

sdrabenh commented May 2, 2022

gmao-rreichle May 2, 2022

gmao-rreichle May 2, 2022

gmao-rreichle May 2, 2022

saraqzhang commented May 3, 2022

gmao-rreichle commented May 3, 2022

sdrabenh commented May 3, 2022

gmao-rreichle commented May 3, 2022

mathomp4 left a comment

		#print ("nxonly option: $nxonly\n");
		#print ("landonly option: $nxonly\n");

		ecbuild_add_executable (TARGET stats.x SOURCES stats.F90)
		ecbuild_add_executable (TARGET statsNx.x SOURCES statsNx.F90)

add fstats customized for screen-level Q and T with "land only" option #246

add fstats customized for screen-level Q and T with "land only" option #246

Conversation

saraqzhang commented Feb 23, 2022

saraqzhang commented Feb 23, 2022

saraqzhang commented Mar 10, 2022

gmao-rreichle commented Mar 22, 2022

gmao-rreichle Mar 31, 2022

Choose a reason for hiding this comment

gmao-rreichle Mar 31, 2022

Choose a reason for hiding this comment

saraqzhang commented Apr 4, 2022

gmao-rreichle Apr 4, 2022

Choose a reason for hiding this comment

saraqzhang Apr 4, 2022

Choose a reason for hiding this comment

sdrabenh commented May 2, 2022

gmao-rreichle May 2, 2022

Choose a reason for hiding this comment

gmao-rreichle May 2, 2022

Choose a reason for hiding this comment

gmao-rreichle May 2, 2022

Choose a reason for hiding this comment

saraqzhang commented May 3, 2022

gmao-rreichle commented May 3, 2022

sdrabenh commented May 3, 2022

gmao-rreichle commented May 3, 2022

mathomp4 left a comment

Choose a reason for hiding this comment