Implement diurnal ozone downscaling #2496

adrifoster · 2024-04-26T20:52:18Z

Description of changes

Adds the capability to downscale daily or multi-day ozone into sub-daily ozone input (DATM or from CAM).

As laid out in CTSM Issue #270 we want to downscale input ozone partial pressure (mol/mol) temporally if we receive a multi-day average input ozone (usually in DATM mode).

We have the infrastructure laid out for CAM and DATM to inform CTSM which type of input we are receiving (i.e., 'multiday_average' or 'subdaily'). We only apply this downscaling when receiving multi-day average ozone.

Specific notes

A gridded file provided by @lkemmons for a diurnal ozone variation factor is read in via a new ozone streams module. This streams file is then used to downscale the ozone provided by DATM or otherwise.

Inside the existing CalcOzoneUptake routine, if we are using multi-day average ozone, interpolate/downscale the forc_o3 array using the new Interp subroutine.

Right now we are assuming that the units of the input diurnal file are going to be in seconds, and that the file covers the whole day.

There is no need to assume that the dimension will be 1-24 (i.e. on the hour) or that the dimensions be equal intervals.

The file is read in using a new src/share_esmf/diurnalOzoneStreamMod module, with associated updates to bld/CLMBuildNamelist.pm, bld/namelist_files/namelist_defaults_ctsm.xml and bld/namelist_files/namelist_definition_csvm.xml

Right now the new variable use_do3_streams is set in the user_nl_clm file. And the other namelist variables (stream_fldfilename_do3, stream_meshfile_do3, and do3_mapalgo) are inside the lnd_in file under a do3_streams namelist.

I'm not sure setting up the use_do3_streams namelist variable is the best way to go about this, since we essentially want this feature on whenever the ozone frequency is 'multiday_average' @billsacks what do you think?

Right now we are hard-coding the stream_var_name, and stream_lev_dimname. I think this is okay. Otherwise, we would want them to be namelist variables that get read in.

These updates do change answers, namely ozone uptake is decreased (@danicalombardozzi does this make sense?)

ozone forcing for non-diurnal vs. diurnal for the first simulation day:

ozone uptake for the same day:

ozone uptake summed up over the year:

@danicalombardozzi i'm not sure about the units for these ozone history variables, they are in mol/m2/time step... do we want to convert them to a per second? Or is this already done somewhere?

Contributors other than yourself, if any:
@billsacks @danicalombardozzi @lkemmons

CTSM Issues Fixed (include github issue #): Fixes #270

Are answers expected to change (and if so in what way)? Yes, see above. If turned on will modify ozone uptake. If ozone damage and ozone downscaling are not on, this will have no effect.

Any User Interface Changes (namelist or namelist defaults changes)? Yes, adds new namelist options for do3_streams. We also need to decide if we want to only optionally do this, or automatically do this all the time if we have multi-day ozone.

Testing performed, if any:

See above, will also do system testing

adrifoster · 2024-04-26T21:05:25Z

TL; DR:

The big thing we still need to decide is how to implement the namelist options and whether we want to automatically downscale ozone any time we get multi-day average, or only optionally do this if

we have multi-day average ozone
the user requests downscaling via "use_do3_streams" in the user_nl_clm file

My vote: just automatically do it. Or optionally turn it off

slevis-lmwg

@adrifoster I looked at your PR and came up with a comment about indentation. If I were nit-picky, I would have pointed out other, aesthetic, indentation issues, but I'm not :-))

Otherwise, this looks great and it seems straightforward enough that I'm comfortable approving without further follow-up.

A question / comment seeking confirmation: I think this is the land model's first example of a stream with an interp function to take data to subdaily, right?

src/biogeophys/OzoneMod.F90

adrifoster · 2024-05-08T17:18:09Z

I looked at your PR and came up with a comment about indentation. If I were nit-picky, I would have pointed out other, aesthetic, indentation issues, but I'm not :-))

I'd like to be consistent, could you point out the other nit-picky things?

adrifoster · 2024-05-08T17:18:27Z

A question / comment seeking confirmation: I think this is the land model's first example of a stream with an interp function to take data to subdaily, right?

Yes I believe that is correct.

adrifoster · 2024-05-08T17:27:16Z

All tests PASS except for expected fails, and NLCOMP diffs (which is expected because of the namelist changes I made)

11:16 $ ./cs.status.fails -c NLCOMP
0506-190200de_gnu: 61 tests
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-default--clm-NEON-NIWO SHAREDLIB_BUILD time=14 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-default--clm-NEON-NIWO RUN time=44 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM SHAREDLIB_BUILD time=9 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM RUN time=92 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL SHAREDLIB_BUILD time=10 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL RUN time=29 (UNEXPECTED: expected FAIL)

 
0506-190200de_int: 130 tests
    FAIL ERP_P256x2_Ld30.f45_f45_mg37.I2000Clm60FatesRs.derecho_intel.clm-mimicsFatesCold RUN time=71 (EXPECTED FAILURE)
    PEND ERP_P256x2_Ld30.f45_f45_mg37.I2000Clm60FatesRs.derecho_intel.clm-mimicsFatesCold COMPARE_base_rest
    FAIL ERP_P64x2_Ly3.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-irrig_o3falk_reduceOutput COMPARE_base_rest
    FAIL ERP_P64x2_Ly3.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-irrig_o3falk_reduceOutput BASELINE ctsm5.2.003: DIFF
    FAIL ERS_D_Ld15.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesColdTwoStream COMPARE_base_rest (EXPECTED FAILURE)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_intel.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO SHAREDLIB_BUILD time=10 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_intel.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO RUN time=37 (UNEXPECTED: expected FAIL)

 
0506-190200de_nvh: 3 tests
    FAIL SMS_D.f10_f10_mg37.I2000Clm60BgcCrop.derecho_nvhpc.clm-crop SHAREDLIB_BUILD time=278 (EXPECTED FAILURE)

slevis-lmwg

@adrifoster I added a few "nit" comments about indentation since you asked.

src/biogeophys/DiurnalOzoneType.F90

src/biogeophys/OzoneMod.F90

billsacks

Thank you very much for your great work on this @adrifoster ! This feels well-designed and well-written, and I appreciate the time you spent working on this feature - both before you went on leave and then the final work you did to finalize it more recently. I especially appreciate the care you took in various places to document things, call out questions you have, etc. I'm sorry it has taken me so long to review it!

I have a number of comments below. I think most of them are pretty small and will hopefully be easy to address. There are maybe a few that might take a bit more thought, such as related to the interpolation algorithm, but nothing that should take any significant rework: the overall structure of this seems great! I'm happy to talk through any of this if it would help.

billsacks · 2024-07-25T17:46:27Z

bld/CLMBuildNamelist.pm

+          if ($opts->{'driver'} ne "nuopc") {
+            $log->fatal_error("Cannot use do3_streams=.true. with MCT.");
+          }


This check can probably be removed now that mct is no longer supported.

billsacks · 2024-07-25T17:49:00Z

bld/CLMBuildNamelist.pm

+          add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'stream_fldfilename_do3',
+                      'hgrid'=>"360x720cru" );
+          add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'stream_meshfile_do3',
+                      'hgrid'=>"360x720cru" );
+          add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'do3_mapalgo',
+                      'hgrid'=>$nl_flags->{'res'} );


Is hgrid actually needed as an attribute to these add_default calls? It is confusing to me because the hgrid you're specifying doesn't match the grid of the files (based on the files given in namelist_defaults). And since there is only a single option in the namelist_defaults, I'm thinking that you probably don't need to specify hgrid (which I would think would be to choose between multiple options) - but I could be misunderstanding.

billsacks · 2024-07-25T18:03:30Z

bld/CLMBuildNamelist.pm

+  # input file for creating diurnal ozone from >daily data
+  my ($opts, $nl_flags, $definition, $defaults, $nl, $physv) = @_;
+
+      add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'use_do3_streams');


As you note in the design document, I'm not sure there's a use case where a user would want to change use_do3_streams. I'd suggest removing that and just basing the logic on atm_ozone_frequency. (You could potentially still avoid adding the stream definition stuff if ozone is off... not sure if it's worth doing that or not.)

billsacks · 2024-07-25T18:03:47Z

bld/namelist_files/namelist_defaults_ctsm.xml

@@ -1732,6 +1732,12 @@ lnd/clm2/surfdata_esmf/NEON/surfdata_1x1_NEON_TOOL_hist_78pfts_CMIP6_simyr2000_c
 <lai_mapalgo  hgrid="1x1_asphaltjungleNJ" >nn</lai_mapalgo>
 <lai_mapalgo  hgrid="5x5_amazon"          >nn</lai_mapalgo>

+<!-- do3 streams namelist defaults -->
+<use_do3_streams>.true.</use_do3_streams>
+<stream_fldfilename_do3>/glade/work/afoster/ozone_update/ozone_damage_files/converted/diurnal_factor_O3surface_ssp370_2015-2024_c20220502.nc</stream_fldfilename_do3>


This file should be moved into inputdata.

billsacks · 2024-07-25T20:29:39Z

doc/design/design_doc_diurnal_ozone.md

@@ -0,0 +1,116 @@
+# Software Design Documentation


Thank you for writing this design doc and adding it here! This is especially helpful when coming back to this a couple of years later like I'm doing now!!!

billsacks · 2024-07-25T20:43:28Z

doc/design/design_doc_diurnal_ozone.md

+
+We add an instance of the `diurnal_ozone_anom_type` as an attribute of the `ozone_base_type`.
+
+Within the `Init` method of the `ozone_base_type`, we read the `drv_flds_in` file to get the `atom_ozone_frequency_val`. If this value is `atm_ozone_frequency_multiday_average` then we initialize and read in the o3 anomaly data by calling the `read_O3_stream` described above. *Do we need an else decision here?*


Good question about the else. I have added a comment on this below, in the relevant part of the code.

billsacks · 2024-07-26T17:12:05Z

src/biogeophys/DiurnalOzoneType.F90

+      do t = 1, this%ntimes
+        if (real(tod) <= this%time_arr(t)) then
+          exit
+        end if
+      end do


Is it possible to have real(tod) > this%time_arr(this%ntimes)? My sense is that this case isn't currently handled, but I'm not sure if it's possible for it to arise based on the times on the file.

billsacks · 2024-07-26T17:18:24Z

src/biogeophys/DiurnalOzoneType.F90

+      forc_o3_down(begg:endg) = forc_o3(begg:endg)*               &
+        ((this%o3_anomaly_grc(begg:endg, t_prev)*tdiff_start +    &
+        this%o3_anomaly_grc(begg:endg, t_prev)*tdiff_end)/tdiff)


Is there a typo here? You use t_prev as the index for both; should one of them be t? I haven't thought carefully about which one should be t other than thinking: in the edge case where tod == this%time_arr(t), then I think we want to use the anomaly from time t exactly, right? I guess one other thing I'm thinking about here as I start to think about this more is that I wonder if the names of the tdiff_start and tdiff_end variables should be swapped: I haven't thought about this carefully, but it seems like tdiff_start gives the difference from the end of the time interval, which seems a bit counter-intuitive... but maybe I'm seeing this wrong.

billsacks · 2024-07-26T17:21:58Z

src/biogeophys/OzoneMod.F90

+    call restartvar(ncid=ncid, flag=flag, varname='o3force', xtype=ncd_double, &
+         dim1name='gridcell', &
+         long_name='ozone forcing', units='mol mol^-1', &
+         readvar=readvar, interpinic_flag='interp', data=this%o3force_grid)


I haven't looked at this carefully, but I just want to double-check: does this truly need to be on the restart file? Or is it / can it be recalculated at the start of the run?

billsacks · 2024-07-26T17:23:55Z

src/biogeophys/OzoneMod.F90

-    integer  :: p              ! patch index
-    integer  :: c              ! column index
-    integer  :: g              ! gridcell index
+    real(r8)          :: forc_o3_down(bounds%begg:bounds%endg) ! downscaled ozone partial pressure (mol/mol)


Is there a reason for this intermediate forc_o3_down as opposed to putting the result directly in the forc_o3_grid variable? If keeping forc_o3_down, I'd suggest changing its name to forc_o3_downscaled (I assume that's what 'down' is short for?) (rationale: 'down' implies to me 'up' vs. 'down', which is a somewhat reasonable interpretation of the name for a forcing, giving a sign convention, so I'd want to make it clear that that is not the meaning here).

adrifoster and others added 30 commits September 1, 2022 14:29

add new diurnal ozone mod

f00dcc1

add do3 streams namelist info

fb2ba21

move to all lower case

37fed71

Merge branch 'master' into diurnal_ozone

898cead

fix namelist variables

9079404

update fieldlist names

743bfa0

update bld

40110ba

fix do3 streams

01c3efa

create diurnalozonetype

ea81976

remove notes

c04e1fd

add call to shr_ozone_readnl

1abfed3

move frequency flag to OzoneModType

eb989a4

typo

53b6008

fix iulog error

46cd241

fix private issue

57ab73a

fix use statement

56792c2

add init call

67d01db

remove ntimes

1e1a540

checking includes

ce71f95

typo fix?

ab88f6a

update ozone type name

734ea15

fix circular dependency

5617195

adding introplation

09d3c2f

Merge branch 'master' into diurnal_ozone

f651f99

lots of updates, design doc

f214f12

add design doc

06a6271

previous updates

0c3ad39

merge in latest master

51f644f

remove weird files?

f85d84f

remove more weird files

673b90a

adrifoster and others added 9 commits April 24, 2024 15:34

updating Interp function

f2d58ca

bug fix

0ecd18b

various updates to actual read namelist args

f62bf19

final updates

b017013

remove ws diffs from buildnamelist

526385b

remove ws diffs

541d806

more ws

0586a7a

even more ws

dfe6489

Merge branch 'master' into diurnal_ozone

314e66d

adrifoster requested review from ekluzek, slevis-lmwg and billsacks April 26, 2024 20:53

adrifoster added 4 commits May 6, 2024 10:44

Merge branch 'master' into diurnal_ozone

87c2183

change logic for use_do3_streams

f1778be

fix assert

8875fba

Merge branch 'master' into diurnal_ozone

752870a

slevis-lmwg approved these changes May 7, 2024

View reviewed changes

src/biogeophys/OzoneMod.F90 Outdated Show resolved Hide resolved

update indentation

ebcdeda

slevis-lmwg reviewed May 8, 2024

View reviewed changes

src/biogeophys/DiurnalOzoneType.F90 Outdated Show resolved Hide resolved

src/biogeophys/OzoneMod.F90 Outdated Show resolved Hide resolved

src/biogeophys/OzoneMod.F90 Outdated Show resolved Hide resolved

samsrabin mentioned this pull request May 9, 2024

Update externals to cesm2_3_beta17 in preparation for rm externals #2493

Closed

fixing indentation inconsistencies

bfbe350

ekluzek mentioned this pull request Jun 20, 2024

Update the drv_flds namelist definition file in CTSM to work with latest CAM/cmeps #2170

Open

billsacks requested changes Jul 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement diurnal ozone downscaling #2496

Implement diurnal ozone downscaling #2496

adrifoster commented Apr 26, 2024

adrifoster commented Apr 26, 2024

slevis-lmwg left a comment

adrifoster commented May 8, 2024

adrifoster commented May 8, 2024

adrifoster commented May 8, 2024

slevis-lmwg left a comment

billsacks left a comment

billsacks Jul 25, 2024

billsacks Jul 25, 2024

billsacks Jul 25, 2024

billsacks Jul 25, 2024

billsacks Jul 25, 2024

billsacks Jul 25, 2024

billsacks Jul 26, 2024

billsacks Jul 26, 2024

billsacks Jul 26, 2024

billsacks Jul 26, 2024


		We add an instance of the `diurnal_ozone_anom_type` as an attribute of the `ozone_base_type`.

		Within the `Init` method of the `ozone_base_type`, we read the `drv_flds_in` file to get the `atom_ozone_frequency_val`. If this value is `atm_ozone_frequency_multiday_average` then we initialize and read in the o3 anomaly data by calling the `read_O3_stream` described above. Do we need an else decision here?

Implement diurnal ozone downscaling #2496

Are you sure you want to change the base?

Implement diurnal ozone downscaling #2496

Conversation

adrifoster commented Apr 26, 2024

Description of changes

Specific notes

adrifoster commented Apr 26, 2024

slevis-lmwg left a comment

Choose a reason for hiding this comment

adrifoster commented May 8, 2024

adrifoster commented May 8, 2024

adrifoster commented May 8, 2024

slevis-lmwg left a comment

Choose a reason for hiding this comment

billsacks 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