TUV-x photolysis #1471
TUV-x photolysis #1471
Conversation
modified: .gitmodules modified: cime_config/buildlib modified: libraries/tuv-x (new commits)
src/chemistry/mozart/chemistry.F90
Outdated
| call mee_ion_final() | ||
| call rate_diags_final() | ||
| call species_sums_final() | ||
| call tuvx_finalize() |
There was a problem hiding this comment.
Could maybe wrap this within if (tuvx_active) to be consistent with the other calls
| !----------------------------------------------------------------- | ||
| ! ... get calculated photolysis rates from TUV-x | ||
| !----------------------------------------------------------------- | ||
| call tuvx_get_photo_rates( state, pbuf, ncol, lchnk, zmid, zint, & |
There was a problem hiding this comment.
It looks like the only place tuvx_get_photo_rates uses state is to get state%ncol in line ~679, but ncol is already passed in here. Maybe state does not have to be passed in to call tuvx_get_photo_rates
| subroutine gas_phase_chemdr(lchnk, ncol, imozart, q, & | ||
| phis, zm, zi, calday, & | ||
| tfld, pmid, pdel, pint, rpdel, rpdeldry, & | ||
| cldw, troplev, troplevchem, & | ||
| ncldwtr, ufld, vfld, & | ||
| delt, ps, & | ||
| fsds, ts, asdir, ocnfrac, icefrac, & | ||
| precc, precl, snowhland, ghg_chem, latmapback, & | ||
| drydepflx, wetdepflx, cflx, fire_sflx, fire_ztop, nhx_nitrogen_flx, noy_nitrogen_flx, & | ||
| use_hemco, qtend, pbuf) | ||
| drydepflx, wetdepflx, cflx, fire_sflx, fire_ztop, & | ||
| nhx_nitrogen_flx, noy_nitrogen_flx, & | ||
| use_hemco, qtend, pbuf, state) |
There was a problem hiding this comment.
These changes appear to be unnecessary as state was already previously passed in as the first argument, and was moved to the last one for some reason.
| use mo_chm_diags, only : chm_diags, het_diags | ||
| use perf_mod, only : t_startf, t_stopf | ||
| use gas_wetdep_opts, only : gas_wetdep_method | ||
| use physics_types, only : physics_state |
There was a problem hiding this comment.
This line is a duplicate of line 318 and does not need to be added again
There was a problem hiding this comment.
The duplicate line has been removed
src/chemistry/mozart/chemistry.F90
Outdated
| call gas_phase_chemdr(lchnk, ncol, imozart, state%q, & | ||
| state%phis, state%zm, state%zi, calday, & | ||
| state%t, state%pmid, state%pdel, state%pint, state%rpdel, state%rpdeldry, & | ||
| cldw, tropLev, tropLevChem, ncldwtr, state%u, state%v, chem_dt, state%ps, & | ||
| fsds, cam_in%ts, cam_in%asdir, cam_in%ocnfrac, cam_in%icefrac, & | ||
| cam_out%precc, cam_out%precl, cam_in%snowhland, ghg_chem, state%latmapback, & | ||
| drydepflx, wetdepflx, cam_in%cflx, cam_in%fireflx, cam_in%fireztop, & | ||
| nhx_nitrogen_flx, noy_nitrogen_flx, use_hemco, ptend%q, pbuf ) | ||
| nhx_nitrogen_flx, noy_nitrogen_flx, use_hemco, ptend%q, pbuf, state ) |
There was a problem hiding this comment.
These changes appear to be unnecessary as state was already previously passed in as the first argument, and was moved to the last one for some reason.
src/chemistry/mozart/mo_tuvx.F90
Outdated
| integer, parameter :: PROFILE_INDEX_SO2 = 7 ! Sulfur dioxide profile index | ||
| integer, parameter :: PROFILE_INDEX_NO2 = 8 ! Nitrogen dioxide profile index |
There was a problem hiding this comment.
These appear to be non-public and unused, can I ask if these are intended to be not implemented?
src/chemistry/mozart/mo_tuvx.F90
Outdated
| ! Definition of the MS93 wavelength grid TODO add description of this | ||
| integer, parameter :: NUM_BINS_MS93 = 4 | ||
| real(kind=r8), parameter :: WAVELENGTH_EDGES_MS93(NUM_BINS_MS93+1) = & | ||
| (/ 181.6_r8, 183.1_r8, 184.6_r8, 190.2_r8, 192.5_r8 /) |
There was a problem hiding this comment.
Missed TODO here about the description here
There was a problem hiding this comment.
added a brief description
src/chemistry/mozart/mo_tuvx.F90
Outdated
| deallocate( cam_grids ) | ||
| deallocate( cam_profiles ) | ||
| deallocate( cam_radiators ) | ||
| deallocate( wavelength ) |
There was a problem hiding this comment.
Since wavelength => cam_grids%get_grid(...) I would suggest switching the deallocate order for safety
| deallocate( cam_grids ) | |
| deallocate( cam_profiles ) | |
| deallocate( cam_radiators ) | |
| deallocate( wavelength ) | |
| deallocate( wavelength ) | |
| deallocate( cam_grids ) | |
| deallocate( cam_profiles ) | |
| deallocate( cam_radiators ) |
|
|
||
| host_profile => profiles%get_profile( "temperature", "K" ) | ||
| this%profiles_( PROFILE_INDEX_TEMPERATURE ) = this%core_%get_updater( host_profile, found ) | ||
| call assert( 418735162, found ) |
There was a problem hiding this comment.
Here and throughout the file, there are many calls to musica_assert::assert with seemingly random numbers for unique error codes. It seems to be consistent with what's done elsewhere in MUSICA, but I would request to at least have a comment somewhere that these numbers are just unique error codes and don't have any particular meaning other than being unique, and this is how the MUSICA library handles errors. Thanks!
src/chemistry/mozart/mo_tuvx.F90
Outdated
| ! || || | ||
| ! || | ||
| ! ----------------- i_int = pver | ||
| ! ================= i_imd = pver ------------------ i_int = 2 |
There was a problem hiding this comment.
typo here:
| ! ================= i_imd = pver ------------------ i_int = 2 | |
| ! ================= i_mid = pver ------------------ i_int = 2 |
deleted: src/chemistry/mozart/mo_setz.F90 deleted: src/chemistry/mozart/mo_tuv_inti.F90 modified: src/chemistry/mozart/chemistry.F90 modified: src/chemistry/mozart/mo_gas_phase_chemdr.F90 modified: src/chemistry/mozart/mo_tuvx.F90
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Thanks for bringing this in @fvitt! I had some cleanup requests in the python code, but otherwise only had a couple other comments. Hopefully none of my requests will cause any issues, but if you find that they do please let me know!
| carma_maxretries = 40 | ||
| ubc_specifier = 'T->MSIS', 'Q->2.d-8vmr', 'CH4->2.d-10vmr', 'H->MSIS', 'N->MSIS', 'O->MSIS', 'O2->MSIS', 'H2->TGCM', 'NO->SNOE' | ||
| tuvx_active = .true. | ||
| tuvx_config_path = '$SRCROOT/src/chemistry/pp_waccm_ma_sulfur/tuvx_config.json' |
There was a problem hiding this comment.
Should we use $COMP_ROOT_DIR_ATM here instead of $SRCROOT? I am only asking because the other tests using TUV-x appear to use $COMP_ROOT_DIR_ATM.
| 'QCO2ext','TCO2ext','PCO2ext','CO2_ext','N2_ext','O_ext','O2_ext' | ||
|
|
||
| tuvx_active = .true. | ||
| tuvx_config_path = '$COMP_ROOT_DIR_ATM/src/chemistry/pp_waccm_ma_mam5/tuvx_config.json' |
There was a problem hiding this comment.
Just-double checking that the config file under pp_waccm_ma_mam5 can also be used for a MAM4 test?
| ############################################################################### | ||
|
|
||
| rc, out, err = run_cmd(command, from_dir=working_dir, verbose=True) | ||
| expect(rc == 0, "Command {} failed with rc={}".format(command, rc)) |
There was a problem hiding this comment.
I would add the out and err info to the expect call here as well (and convert it to an f-string):
| expect(rc == 0, "Command {} failed with rc={}".format(command, rc)) | |
| expect(rc == 0, f"Command {command} failed with rc={rc} out={out} err={err}") | |
| args = "-DCONVERT_TO_MAKE=ON " | ||
| args += "-DCASEROOT={} ".format(caseroot) | ||
| args += "-DCOMPILER={} ".format(build.get_value("COMPILER")) | ||
| args += "-DOS={} ".format(build.get_value("OS")) | ||
| args += "-DMACH={} ".format(case.get_value("MACH")) | ||
| args += "-DCMAKE_C_COMPILER_WORKS=1 " | ||
| args += "-DCMAKE_Fortran_COMPILER_WORKS=1 " | ||
| args += "-DCMAKE_CXX_COMPILER_WORKS=1 " | ||
| args += "-DDEBUG={} ".format(build.get_value("DEBUG")) | ||
| cmd = "cmake {} .".format(args) | ||
| rc, out, err = run_cmd(cmd, combine_output=True, from_dir=macro_path) | ||
| expect(rc == 0, "Command {} failed with rc={} out={} err={}".format(cmd, rc, out, err)) |
There was a problem hiding this comment.
I would convert these all to f-strings:
| args = "-DCONVERT_TO_MAKE=ON " | |
| args += "-DCASEROOT={} ".format(caseroot) | |
| args += "-DCOMPILER={} ".format(build.get_value("COMPILER")) | |
| args += "-DOS={} ".format(build.get_value("OS")) | |
| args += "-DMACH={} ".format(case.get_value("MACH")) | |
| args += "-DCMAKE_C_COMPILER_WORKS=1 " | |
| args += "-DCMAKE_Fortran_COMPILER_WORKS=1 " | |
| args += "-DCMAKE_CXX_COMPILER_WORKS=1 " | |
| args += "-DDEBUG={} ".format(build.get_value("DEBUG")) | |
| cmd = "cmake {} .".format(args) | |
| rc, out, err = run_cmd(cmd, combine_output=True, from_dir=macro_path) | |
| expect(rc == 0, "Command {} failed with rc={} out={} err={}".format(cmd, rc, out, err)) | |
| args = "-DCONVERT_TO_MAKE=ON " | |
| args += f"-DCASEROOT={caseroot} " | |
| args += f"-DCOMPILER={build.get_value("COMPILER")} " | |
| args += f"-DOS={build.get_value("OS")} " | |
| args +=f "-DMACH={case.get_value("MACH")} " | |
| args += "-DCMAKE_C_COMPILER_WORKS=1 " | |
| args += "-DCMAKE_Fortran_COMPILER_WORKS=1 " | |
| args += "-DCMAKE_CXX_COMPILER_WORKS=1 " | |
| args += f"-DDEBUG={build.get_value("DEBUG")} " | |
| cmd = f"cmake {args} ." | |
| rc, out, err = run_cmd(cmd, combine_output=True, from_dir=macro_path) | |
| expect(rc == 0, f"Command {cmd} failed with rc={rc} out={out} err={err}") |
| arg_dict = _cmake_default_args(caseroot) | ||
| if build.get_value("MPILIB") == "mpi-serial": | ||
| cmake_args = "-DCMAKE_Fortran_COMPILER={} ".format(arg_dict["SFC"]) | ||
| else: | ||
| cmake_args = "-DCMAKE_Fortran_COMPILER={} ".format(arg_dict["MPIFC"]) | ||
| cmake_args += "-DTUVX_ENABLE_MPI:BOOL=TRUE " | ||
| if case.get_value("DEBUG"): | ||
| cmake_args += "-DCMAKE_BUILD_TYPE=Debug " | ||
| else: | ||
| cmake_args += "-DCMAKE_BUILD_TYPE=Release " | ||
| cmake_args += "-DCMAKE_VERBOSE_MAKEFILE=ON " |
There was a problem hiding this comment.
It looks like much of this Fortran compiler logic is immediately clobbered below. Given that I don't think CAM ever really uses the "mpi-serial" library, I might clean this up like so:
| arg_dict = _cmake_default_args(caseroot) | |
| if build.get_value("MPILIB") == "mpi-serial": | |
| cmake_args = "-DCMAKE_Fortran_COMPILER={} ".format(arg_dict["SFC"]) | |
| else: | |
| cmake_args = "-DCMAKE_Fortran_COMPILER={} ".format(arg_dict["MPIFC"]) | |
| cmake_args += "-DTUVX_ENABLE_MPI:BOOL=TRUE " | |
| if case.get_value("DEBUG"): | |
| cmake_args += "-DCMAKE_BUILD_TYPE=Debug " | |
| else: | |
| cmake_args += "-DCMAKE_BUILD_TYPE=Release " | |
| cmake_args += "-DCMAKE_VERBOSE_MAKEFILE=ON " | |
| arg_dict = _cmake_default_args(caseroot) | |
| cmake_args = "-DCMAKE_VERBOSE_MAKEFILE=ON " | |
| if build.get_value("MPILIB") != "mpi-serial": | |
| cmake_args += "-DTUVX_ENABLE_MPI:BOOL=TRUE " | |
| if case.get_value("DEBUG"): | |
| cmake_args += "-DCMAKE_BUILD_TYPE=Debug " | |
| else: | |
| cmake_args += "-DCMAKE_BUILD_TYPE=Release " |
| expect(len(corepaths)>0, \ | ||
| "TUV-x not found at {}".format(libroot)) | ||
| expect(len(corepaths)<2, \ | ||
| "Multiple TUV-x versions found at {}".format(libroot)) | ||
| expect(os.path.exists(corepaths[0]), \ | ||
| "TUV-x install directory not found at {}".format(corepaths[0])) |
There was a problem hiding this comment.
I would convert these error messages to f-strings:
| expect(len(corepaths)>0, \ | |
| "TUV-x not found at {}".format(libroot)) | |
| expect(len(corepaths)<2, \ | |
| "Multiple TUV-x versions found at {}".format(libroot)) | |
| expect(os.path.exists(corepaths[0]), \ | |
| "TUV-x install directory not found at {}".format(corepaths[0])) | |
| expect(len(corepaths)>0, \ | |
| f"TUV-x not found at {libroot}" | |
| expect(len(corepaths)<2, \ | |
| f"Multiple TUV-x versions found at {libroot}" | |
| expect(os.path.exists(corepaths[0]), \ | |
| f"TUV-x install directory not found at {corepaths[0]}" |
| return corepaths[0] | ||
|
|
||
| ############################################################################### | ||
| def _tuvx_package_dir(libroot): |
There was a problem hiding this comment.
I don't think this function is actually used anywhere (?).
| # copy geos-chem config files to rundir if using geos-chem chemistry | ||
| # ----------------------------------------------------- | ||
|
|
||
There was a problem hiding this comment.
Is there a reason for the extra whitespace here?
| end if | ||
| call aurora_register() | ||
| endif | ||
| call tuvx_register( ) |
There was a problem hiding this comment.
Is there a reason to call tuvx_register() if tuvx_active is .false.? If not then I might update this to be:
| call tuvx_register( ) | |
| if( tuvx_active ) call tuvx_register( ) |
| character(len=2) :: mstring | ||
| character(len=7) :: jstring | ||
| logical :: do_jeuv | ||
| logical, save :: is_initialized = .false. |
There was a problem hiding this comment.
Nit-pick, but can we possibly move this up to be a protected module-level variable? I generally wouldn't expect saved variables to exist within a subroutine.
3 participants
This provides the option to use the TUV-x library to compute photo-chemical rates in-line as an alternative to the traditional table look up method.
Closes #757