cam6_3_027: Bug fixes for WACCMX #382

fvitt · 2021-05-26T18:59:12Z

Misc bug fixes for WACCMX:

array out of bounds issue in electric potential solver when compiled with some compilers (array out of bounds errors in WACCMX e-potential direct solver #363)
error writing physics buffer fields to restart file in large simulations (Error writing pbuf fields to restart file at ne120 282 layer resolution #376)
initialization of ion and electron temperatures when not read from IC file (WACCMX ion and electron temperatures are initialized to zero when not read from IC file #377)
do not attempt to advect N2 when SE CSLAM is used (addfld error when SE CSLAM is used in WACCMX #379)

fixes #363
fixes #376
fixes #377
fixes #379

modified: Externals.cfg modified: cime_config/testdefs/testlist_cam.xml new file: cime_config/testdefs/testmods_dirs/cam/outfrq3s_edyngrid_160x193/shell_commands new file: cime_config/testdefs/testmods_dirs/cam/outfrq3s_edyngrid_160x193/user_nl_cam new file: cime_config/testdefs/testmods_dirs/cam/outfrq3s_edyngrid_160x193/user_nl_clm new file: cime_config/testdefs/testmods_dirs/cam/outfrq3s_edyngrid_320x385/shell_commands new file: cime_config/testdefs/testmods_dirs/cam/outfrq3s_edyngrid_320x385/user_nl_cam new file: cime_config/testdefs/testmods_dirs/cam/outfrq3s_edyngrid_320x385/user_nl_clm modified: src/ionosphere/waccmx/edyn_mudcom.F90 modified: src/ionosphere/waccmx/edyn_solve.F90

modified: bld/namelist_files/namelist_defaults_cam.xml modified: src/dynamics/se/dyn_comp.F90 modified: src/utils/physconst.F90

…g xlrg restart files modified: src/physics/cam/physpkg.F90 modified: src/physics/cam/waccmx_phys_intr.F90 modified: src/physics/waccmx/ion_electron_temp.F90 modified: src/utils/cam_map_utils.F90

Externals.cfg

src/ionosphere/waccmx/edyn_mudcom.F90

src/physics/cam/waccmx_phys_intr.F90

src/utils/physconst.F90

nusbaume

Looks good to me! Just a couple optional requests to change the log output, and one question related to possibly skipping a file read.

nusbaume · 2021-07-06T22:11:25Z

src/physics/waccmx/ion_electron_temp.F90

+          if (masterproc) write(iulog,*) 'ion_electron_temp_inidat: Not able to read temperature from IC file.' &
+                                       //' Set TElec to NaN'


Would it make sense to add to this message stating that both TElec and TIon will be set to phys_state%t, which I assume is the neutral temperature?

That would make sense.

nusbaume · 2021-07-06T22:16:50Z

src/physics/waccmx/ion_electron_temp.F90

          call infld( 'T',ncid_ini,dim1name, 'lev', dim2name, 1, pcols, 1, pver, begchunk, endchunk, &
               tI, found, gridname='physgrid')
       endif
+       if (found) then


If initialized_TiTe is false (i.e. neither the election temperatures nor neutral temperatures were found above) , then should one even bother trying to read in the ion temperatures, given that they will be overwritten by ion_electron_temp_timestep_init anyways?

nusbaume · 2021-07-06T22:17:11Z

src/physics/waccmx/ion_electron_temp.F90

+       if (found) then
+          call pbuf_set_field(pbuf2d, index_ti, tI)
+       else
+          if (masterproc) write(iulog,*) 'ion_electron_temp_inidat: Not able to read temperature from IC file.' &


Would it make sense to add to this message stating that both TElec and TIon will be set to phys_state%t, which I assume is the neutral temperature?

Makes sense.

gold2718

I have a couple of comments and questions but nothing that looks like it needs fixing (but please look at the question in election_ion_temp.F90).

gold2718 · 2021-05-31T22:08:44Z

src/utils/cam_map_utils.F90


    !
-    fileSize = product(filelens)
+    fileSize = product( int(filelens,kind=iMap) )


Ooh, nice catch!

gold2718 · 2021-06-01T04:12:46Z

src/utils/physconst.F90

   water_species_in_air_num = 0
   do i = 1, num_names_max
-      if (.not. LEN(TRIM(dry_air_species(i)))==0) then
+      if ((.not. LEN(TRIM(dry_air_species(i)))==0).and.(TRIM(dry_air_species(i))/='N2')) then


@jtruesdal, @PeterHjortLauritzen,
Have you had a chance to review the changes in this module?
Also, the logic could be cleaner (including use of the LEN_TRIM Fortran intrinsic:

Suggested change

if ((.not. LEN(TRIM(dry_air_species(i)))==0).and.(TRIM(dry_air_species(i))/='N2')) then

if ((LEN_TRIM(dry_air_species(i)) > 0) .and. (TRIM(dry_air_species(i)) /= 'N2')) then

There are other places in this module where LEN(TRIM(xx)) should be replaced with LEN_TRIM(xx).

Yes I have reviewed the code and I am happy with it. @gold2718 Do you have ideas for a cleaner implementation?

@PeterHjortLauritzen just checking if you took a look at the fvitt changes to physconst and approve them (PR #382)?

src/physics/cam/waccmx_phys_intr.F90

gold2718 · 2021-07-08T17:45:53Z

src/physics/waccmx/ion_electron_temp.F90

+          call pbuf_set_field(phys_buffer_chunk,index_te,phys_state(lchnk)%t(:ncol,:),start=(/1,1/), kount=(/ncol,pver/))
+          call pbuf_set_field(phys_buffer_chunk,index_ti,phys_state(lchnk)%t(:ncol,:),start=(/1,1/), kount=(/ncol,pver/))


At line 278, the pbuf_set_field calls are within a if (index_te>0) then block. Why not here?

Good catch.

cacraigucar

When cam6_3_026 is tagged, please merge your code forward. At that point, I will verify that the cime version is okay and submit my approval. I've tentatively slated this PR for cam6_3_027.

jtruesdal · 2021-07-13T16:39:13Z

Accidently posted here. Removed comment.

PeterHjortLauritzen

@jtruesdal yes I approve. Just had some clarifying questions

PeterHjortLauritzen · 2021-07-08T18:28:03Z

src/utils/physconst.F90

      if (masterproc) then
        write(iulog, *) "Dry air composition ",TRIM(dry_air_species(i)),&
-             icnst,thermodynamic_active_species_idx(icnst),&
+             icnst, -1,&


Did you mean to write "icnst -1" to log file?

PeterHjortLauritzen · 2021-07-08T18:29:15Z

src/utils/physconst.F90

   water_species_in_air_num = 0
   do i = 1, num_names_max
-      if (.not. LEN(TRIM(dry_air_species(i)))==0) then
+      if ((.not. LEN(TRIM(dry_air_species(i)))==0).and.(TRIM(dry_air_species(i))/='N2')) then


Yes I have reviewed the code and I am happy with it. @gold2718 Do you have ideas for a cleaner implementation?

src/utils/physconst.F90

cacraigucar

Post merge approval of Externals.cfg not being changed.

fvitt added 5 commits May 25, 2021 14:20

merge in changes from waccmx_cslam branch

24ee806

modified: bld/namelist_files/namelist_defaults_cam.xml modified: src/dynamics/se/dyn_comp.F90 modified: src/utils/physconst.F90

init Te and Ti to T when not read from IC file; fix issue with writin…

4656a71

…g xlrg restart files modified: src/physics/cam/physpkg.F90 modified: src/physics/cam/waccmx_phys_intr.F90 modified: src/physics/waccmx/ion_electron_temp.F90 modified: src/utils/cam_map_utils.F90

adjust test list

9806546

adjust test list

8b760fb

fvitt added BFB bit for bit tag bug-fix This PR was created to fix a specific bug. labels May 26, 2021

fvitt added this to the CESM2.3 milestone May 26, 2021

fvitt requested review from cacraigucar, gold2718 and nusbaume May 26, 2021 18:59

fvitt self-assigned this May 26, 2021

cacraigucar requested changes Jun 24, 2021

View reviewed changes

Externals.cfg Outdated Show resolved Hide resolved

src/ionosphere/waccmx/edyn_mudcom.F90 Show resolved Hide resolved

src/physics/cam/waccmx_phys_intr.F90 Show resolved Hide resolved

src/utils/physconst.F90 Show resolved Hide resolved

cacraigucar requested a review from peverwhee June 28, 2021 22:35

fix for high-res edyn mag grid

6bcaef9

nusbaume approved these changes Jul 7, 2021

View reviewed changes

changes in response to nusbaume's review

c5b45e9

gold2718 approved these changes Jul 8, 2021

View reviewed changes

peverwhee approved these changes Jul 8, 2021

View reviewed changes

check pbuf indices > 0 before setting fields to neutral temp

92fea3a

cacraigucar reviewed Jul 13, 2021

View reviewed changes

cacraigucar changed the title ~~Bug fixes for WACCMX~~ cam6_3_027?: Bug fixes for WACCMX Jul 13, 2021

PeterHjortLauritzen approved these changes Jul 14, 2021

View reviewed changes

fvitt added 3 commits July 19, 2021 09:43

merge to head of cam_development

05dac20

update ChangeLog

3f7c85b

update ChangeLog

a130011

fvitt merged commit 14254c3 into ESCOMP:cam_development Jul 20, 2021

cacraigucar changed the title ~~cam6_3_027?: Bug fixes for WACCMX~~ cam6_3_027: Bug fixes for WACCMX Jul 22, 2021

cacraigucar reviewed Jul 22, 2021

View reviewed changes

		if (masterproc) write(iulog,*) 'ion_electron_temp_inidat: Not able to read temperature from IC file.' &
		//' Set TElec to NaN'

	if ((.not. LEN(TRIM(dry_air_species(i)))==0).and.(TRIM(dry_air_species(i))/='N2')) then
	if ((LEN_TRIM(dry_air_species(i)) > 0) .and. (TRIM(dry_air_species(i)) /= 'N2')) then

		call pbuf_set_field(phys_buffer_chunk,index_te,phys_state(lchnk)%t(:ncol,:),start=(/1,1/), kount=(/ncol,pver/))
		call pbuf_set_field(phys_buffer_chunk,index_ti,phys_state(lchnk)%t(:ncol,:),start=(/1,1/), kount=(/ncol,pver/))

cam6_3_027: Bug fixes for WACCMX #382

cam6_3_027: Bug fixes for WACCMX #382

Uh oh!

Conversation

fvitt commented May 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nusbaume left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gold2718 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cacraigucar left a comment

Choose a reason for hiding this comment

Uh oh!

jtruesdal commented Jul 13, 2021 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PeterHjortLauritzen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cacraigucar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jtruesdal commented Jul 13, 2021 via email •

edited

Loading

PeterHjortLauritzen left a comment •

edited

Loading