cam6_2_005: SE updates; new analytic IC functionality #40

brian-eaton · 2019-11-27T23:20:01Z

addresses issue #37

brian-eaton · 2019-11-29T18:34:31Z

Testing is done. This is ready for commit.

gold2718

A few questions, a few comments, and a few things I think need changing.

gold2718 · 2019-11-30T02:41:39Z

doc/ChangeLog

+cheyenne/intel:
+
+cheyenne/intel/aux_cam:
+
+izumi/nag:
+
+izumi/pgi:


It helps to indicate that the tests were run with no errors (e.g., 'All Pass')

I didn't update the ChangeLog with this info because I don't know that I've done my final testing. I plan to do that after I get a commit slot.

gold2718 · 2019-11-30T05:40:36Z

src/dynamics/fv/dyn_comp.F90

+                 write(iulog,*) sub//': ERROR: model parameters do not match initial dataset parameters'
+                 write(iulog,*)'Model Parameters:    plon = ',plon,' plat = ',plat
+                 write(iulog,*)'Dataset Parameters:  dlon = ',mlon,' dlat = ',mlat


Why is there no masterproc control here?

I don't think there's a need for masterproc control when the messages are in front of an endrun statement. This isn't normal log output.

gold2718 · 2019-11-30T06:01:40Z

src/dynamics/fv/dyn_comp.F90

+      ierr = pio_inq_dimid(fh_topo, 'lon' , lonid)
+      ierr = pio_inq_dimid(fh_topo, 'lat' , latid)
+      ierr = pio_inq_dimlen(fh_topo, lonid , mlon)
+      ierr = pio_inq_dimlen(fh_topo, latid , mlat)
+      if (mlon /= plon .or. mlat /= plat) then
+         write(iulog,*) sub//': ERROR: model parameters do not match initial dataset parameters'
+         write(iulog,*)'Model Parameters:    plon = ',plon,' plat = ',plat
+         write(iulog,*)'Dataset Parameters:  dlon = ',mlon,' dlat = ',mlat
+         call endrun(sub//': ERROR: model parameters do not match initial dataset parameters')
+      end if


It seems that you have a similar check starting at 2989 which could be consolidated into a subroutine.
At the very least, please fix the log and endrun calls to mention the correct file (topo, not IV).

I fixed the error messages to indicate topo file.

gold2718 · 2019-11-30T06:12:10Z

src/dynamics/fv/dyn_comp.F90

+      if (.not. present(m_cnst) .or. .not. present(fh_ini)) then
         call endrun(sub//': ERROR:  m_cnst needs to be present in the'// &
                     ' argument list')
      end if


I do not understand this logic. If m_cnst is not present, you have the original error message.
If m_const is present but fh_ini is not present, you get an error that m_cnst needs to be present but it is present. At the very least, correct the error message.
However, it seems to me that m_cnst is always required in this case and fh_ini is required if readvar is .true.
Am I missing something?

Your interpretation of the logic is correct. I moved the check for fh_ini inside the readvar condition.

gold2718 · 2019-11-30T06:13:54Z

src/dynamics/fv/dyn_comp.F90

+         write(errormsg, *) trim(fieldname),' was not present in the input file.'
+         call endrun('DYN_FIELD_EXISTS: '//errormsg)


I recall that we are supposed to log errors as well as endrun them. Has that rule changed (or been mis-remembered)?

I know the code is not consistent in this regard. I'll suggest that when every task is going to return the same error that the endrun statement is sufficient. That's the case here since every task is looking for the same field in the same file. It seems most useful to include log output when it's possible that only one task will encounter an error, and you don't know which task that will be.

gold2718 · 2019-11-30T23:25:09Z

src/dynamics/tests/initial_conditions/ic_us_standard_atm.F90

+      ncnst = size(m_cnst, 1)
+      do m = 1, ncnst
+         if (m_cnst(m) == 1) then
+            ! No water vapor in Held-Suarez


Shouldn't this say 'No water vapor in standard atmosphere'?

I changed it to ! No water vapor in profile

gold2718 · 2019-12-01T03:01:59Z

src/dynamics/se/dyn_comp.F90

@@ -1076,7 +1160,8 @@ subroutine read_inidat(dyn_in)
   integer,  allocatable            :: m_ind(:)
   real(r8), allocatable            :: dbuf4(:,:,:,:)
   !----------------------------------------------------------------------------
-
+   logical :: lbalance_ps_with_phis=.false.


What is the purpose of this? It does not seem to be used.

gold2718 · 2019-12-01T03:18:23Z

src/dynamics/se/dyn_comp.F90

+         write(errmsg, '(a,3(i0,a))') ': ntrac (',ntrac,') > qsize (',qsize, &
+            ') but < pcnst (',pcnst,')'


No longer writing to log?

This is covered by my new rule: all tasks see the same error, so endrun is sufficient.

gold2718 · 2019-12-01T03:18:30Z

src/dynamics/se/dyn_comp.F90

         call endrun(trim(subname)//errmsg)
      end if
+   else if (qsize < pcnst) then
+      write(errmsg, '(a,2(i0,a))') ': qsize (',qsize,') < pcnst (',pcnst,')'


No longer writing to log?

new rule applies:)

gold2718 · 2019-12-01T03:19:48Z

src/dynamics/se/dyn_comp.F90

+   if (analytic_ic_active()) cnst_start = 2
+
+   ! If using analytic ICs the initial file only needs the horizonal grid
+   ! dimension checked in the case that the file contains constituent mixing


Why? Don't the levels have to match up?

The purpose of that condition is to prevent overwriting an analytic Q field with one from an IC file. We want to allow an IC file to provide all the tracers when using analytic ICs except for Q. The main use case is to use the new standard atmosphere profile to initialize the dynamic state, but to be able to take all the chemical constituents from an initial file. Peter has found that this is a pretty stable way of starting a full physics run on a new grid for which we don't have a balanced IC file.

I am not asking about the condition but about the comment. First, I do not understand the comment; While check_file_layout only checks the horizontal grid, I believe the layers do have to match.

The only thing an initial file must contain is the vertical coordinates. This consistency check is done earlier in dyn_grid_init -> hycoef_init -> hycoef_read.

Before the current changes the IC file would not be checked again if analytic ICs were used. Now we want to allow constituents to be initialized from the IC file if they are available even when the dynamic state is analytic. So we're going to look in the IC file to see whether any constituents except Q are present, and if they are then we need to make sure the horizontal grid is consistent with ncol.

some fixes when modifying url

Update cflx correctly for dms when it is obtained from the ocean

brian-eaton added 11 commits October 28, 2019 11:17

merge SE mods to CAM dev branch

7f59e11

fix external in ic_us_standard_atmosphere.F90

1c5d257

add separate phis_in, phis_out in inic_analytic.F90

3d976fa

add read_phis for FV

bcf3173

modify analytic IC interfaces in SE and EUL

d9b13bd

fix FV process_inidat

0819b45

fixes for test failures

7be9051

mods for waccm, conus

07319a9

mods conus timestep

6247b41

mods for rayleigh friction

ee4cfe2

update to cam6_2_004

675e43f

brian-eaton added enhancement New feature or request cam_development answer changing answer changing tag labels Nov 27, 2019

brian-eaton requested review from gold2718, cacraigucar and nusbaume November 27, 2019 23:20

brian-eaton self-assigned this Nov 27, 2019

gold2718 requested changes Dec 1, 2019

View reviewed changes

cacraigucar approved these changes Dec 2, 2019

View reviewed changes

brian-eaton added 2 commits December 2, 2019 16:00

mods from review comments

7de601f

update ChangeLog

0c5fe23

brian-eaton merged commit a7f193d into ESCOMP:cam_development Dec 4, 2019

brian-eaton deleted the se_cam6_2_004 branch December 4, 2019 00:21

brian-eaton mentioned this pull request Dec 4, 2019

SE updates; new analytic IC functionality. #37

Closed

cacraigucar changed the title ~~Se cam6 2 004~~ cam6_2_005: SE updates; new analytic IC functionality Dec 4, 2019

jedwards4b added a commit to jedwards4b/CAM that referenced this pull request Jun 1, 2024

Merge pull request ESCOMP#40 from ESMCI/fix/change_url

9e5d5f7

some fixes when modifying url

gold2718 added a commit to gold2718/CAM that referenced this pull request Dec 1, 2024

Merge pull request ESCOMP#40 from mvertens/feature/bugfix_dms

8dba2ec

Update cflx correctly for dms when it is obtained from the ocean

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cam6_2_005: SE updates; new analytic IC functionality #40

cam6_2_005: SE updates; new analytic IC functionality #40

brian-eaton commented Nov 27, 2019

brian-eaton commented Nov 29, 2019

gold2718 left a comment

gold2718 Nov 30, 2019

brian-eaton Dec 2, 2019

gold2718 Nov 30, 2019

brian-eaton Dec 2, 2019

gold2718 Nov 30, 2019

brian-eaton Dec 2, 2019

gold2718 Nov 30, 2019

brian-eaton Dec 2, 2019

gold2718 Nov 30, 2019

brian-eaton Dec 2, 2019

gold2718 Nov 30, 2019

brian-eaton Dec 2, 2019

gold2718 Dec 1, 2019

brian-eaton Dec 2, 2019

gold2718 Dec 1, 2019

brian-eaton Dec 2, 2019

gold2718 Dec 1, 2019

brian-eaton Dec 2, 2019

gold2718 Dec 1, 2019

brian-eaton Dec 2, 2019

gold2718 Dec 2, 2019

brian-eaton Dec 3, 2019

		write(errormsg, *) trim(fieldname),' was not present in the input file.'
		call endrun('DYN_FIELD_EXISTS: '//errormsg)

		write(errmsg, '(a,3(i0,a))') ': ntrac (',ntrac,') > qsize (',qsize, &
		') but < pcnst (',pcnst,')'

cam6_2_005: SE updates; new analytic IC functionality #40

cam6_2_005: SE updates; new analytic IC functionality #40

Conversation

brian-eaton commented Nov 27, 2019

brian-eaton commented Nov 29, 2019

gold2718 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment