Refactor rad_constituents, aerosol_optics_cam for portability; extend abstract aerosol interface#1506
Conversation
…hrough 4f plus cleanup (interactive rebase on 64_156) rad_cnst/aerosol interface refactor phases 1 to 3 rad_cnst/aerosol interface refactor 1: make one aerosol property obj per list_idx rad_cnst/aerosol interface refactor 1b: make one aerosol state obj per list_idx rad_cnst/aerosol interface refactor 1c: remove list_idx argument from aerosol property objects rad_cnst/aerosol interface refactor 2a: migrate to aerosol_definition_mod.F90 (non-buildable) aer-gas split in rad_cnst (to test) split non-portable code by feature (to test) continued refactoring of rad_cnst/abstract aerosol interface. rad_cnst/aerosol interface refactor 3: gas-aerosol split; module reorg wip refactor 3c: move indexing-related to aerosol_mmr_cam aerosol abstract interface refactor 4a: prototypes for moving all access to abstract aerosol interface (aer_rad_props; microp_aero testbed) abstract aerosol interface refactor 4b: move initialization of state to avoid wrong ordering; revert bam validation logic in mirop abstract aerosol interface refactor 4c: move runtime to use aerosol_state when possible (excl register and init phases) abstract aerosol interface refactor 4c bugfix: aquaplanet sets; ODV_ diagnostic abstract aerosol interface refactor 4d bugfix: nucleate_ice_cam always active even when aerosol count is 0; implement nlogsig for BAM abstract aerosol interface refactor 4e: cleanup use statements; keyword args for get mmr for clarity; rename init1 init2 for clarity. abstract aerosol interface refactor 4f: cleanup use statements for radiative_aerosol; radiative_aerosol_definitions Continue cleanup of remnant rad_constituents; reorder comments, etc. Fix build for FCARMA (but introduces aerosol_mmr_cam dep into carma_aero/aero_model.F90) Fix OMP bug; cleanup Aerosol list renaming for clarity. Fix build error in var rename Comment cleanup Cleanup of dryrad, num_to_mass_aer leftover in modal, sectional_props. Change to globally held aerosol_state instance (to test threading) Fix build (remnant issue in microp_aero_final)
Draft optics some cleanup; move parameter 1.3 to be shared cleanup aerosol_optics_core Fix build; cleanup Make diag arguments non-optional
…r use in aerosol_optics_core.
fvitt
left a comment
fvitt
left a comment
There was a problem hiding this comment.
I started a review. You have done a lot of nice work on this. I have a couple of questions on some of this.
fvitt
left a comment
fvitt
left a comment
There was a problem hiding this comment.
A few more random things.
| ncol, pver, top_lev, nswbands, nlwbands, numrh, & | ||
| idx_sw_diag, & | ||
| relh(:ncol,:), sulfwtpct(:ncol,:), mass(:ncol,:), crefwsw, crefwlw, & | ||
| geometric_radius=geometric_radius(:ncol,:), & |
There was a problem hiding this comment.
I believe Fortran allows you to pass a null pointer as an actual argument corresponding to an optional pointer dummy argument. I you take advantage of this, the if (associated(geometric_radius)) check could be removed and these 2 calls to aerosol_optics_sw_bin can be combined. This may require some changes in aerosol_optics_core to declare geometric_radius the dummy argument with both pointer and optional attributes.
There was a problem hiding this comment.
Thanks Francis, this is a great idea. I changed to optional, pointer and check for both present and associated in aerosol_optics_core.
I wasn't sure of compiler behavior, so I did some digging. You probably know this so I'm just writing it down for future reference.
The F2008 standard is quite clear on the behavior of passing null() to optional dummy arguments (emphasis mine)
12.5.2.12 Argument presence and restrictions on arguments not present
A dummy argument or an entity that is host associated with a dummy argument is not present if the dummy argument
• does not correspond to an actual argument,
• corresponds to an actual argument that is not present, or
• does not have the ALLOCATABLE or POINTER attribute, and corresponds to an actual argument that
– has the ALLOCATABLE attribute and is not allocated, or
– has the POINTER attribute and is disassociated.
The F2003 standard was not very clear on this so I suspect I need a associated() check to go together with the present() check.
I found an example of this that's currently in CAM
CAM/src/chemistry/utils/modal_aero_calcsize.F90
Lines 1264 to 1271 in 140295a
so I think the compiler support should be solid if both checks are used, so that's what I adopted.
src/physics/cam/aerosol_mmr_cam.F90
Outdated
|
|
||
| public :: aerosol_mmr_init ! allocate zero_cols | ||
| public :: get_cam_idx | ||
| public :: resolve_mode_idx, resolve_bin_idx |
There was a problem hiding this comment.
To be consistent, can resolve_bin_idx be declared public in a separate line?
| public :: resolve_mode_idx, resolve_bin_idx | |
| public :: resolve_mode_idx | |
| public :: resolve_bin_idx |
There was a problem hiding this comment.
Thanks, moved to separate line. Originally as suggested by Simone I tried to merge them together but opted not to within this PR.
src/physics/cam/aerosol_mmr_cam.F90
Outdated
|
|
||
| implicit none | ||
| private | ||
| save |
There was a problem hiding this comment.
The save statement at the module level is redundant. This line can be removed.
| module procedure rad_aer_get_mam_props_by_idx | ||
| end interface | ||
|
|
||
| ! Public routines — aerosol queries (rad_aer_* naming) |
There was a problem hiding this comment.
The dash in line 29 is showing up as some strange characters in xxdiff.
| ! Public routines — aerosol queries (rad_aer_* naming) | |
| ! Public routines -- aerosol queries (rad_aer_* naming) |
There was a problem hiding this comment.
Thanks, removed throughout.
| ! | ||
| ! NOTE: A model is "globally active" if the climate list (list 0) has > 0 entries | ||
| ! for aerosol with that representation, but individual diagnostic lists | ||
| ! may have zero entries — in that case the corresponding properties slot is left null. |
There was a problem hiding this comment.
Line 78 contains strange characters (the "dash").
There was a problem hiding this comment.
Thanks, removed throughout.
| ! Number of aerosol models active at runtime. | ||
| ! Note: Multiple aerosol models can be active at once, e.g., using bulk for volcanic aerosol and modal for others. | ||
| ! When retrieving properties via aerosol_instances_get_props, or creating states from | ||
| ! aerosol_instances_create_states, ensure that the aerosol model matches what is needed (e.g., aero_props%model_is('MAM') == .true.) |
There was a problem hiding this comment.
Some of these lines in the comments seem extra long. Maybe limit the line lengths to 80, or so, characters.
There was a problem hiding this comment.
Thanks, tried to limit to 80 by rearranging the text a bit.
…further for idx_sw_diag duplication loop
| real(r8),intent(out) :: palb(ncol) ! parameterized single scattering albedo | ||
| real(r8),intent(out) :: pasm(ncol) ! parameterized asymmetry factor |
There was a problem hiding this comment.
I noticed these comments were possibly swapped so I just updated them in this PR
2 participants
Motivation for changes:
Purpose of changes (include the issue number and title text for each relevant GitHub issue):
Closes #1504, #1505
This pull request is an extensive refactor of
rad_constituents.F90to facilitate bringing into CAM-SIMA.rad_constituents.F90into aerosol-related modules; the original file will only handle gases. This is because RRTMGP port to CAM-SIMA split off radiatively active aerosols from gases+aerosols.aerosol_mmr_cam.F90, which can be modified to use whichever data structures are in other host models (e.g., the CCPP constituents object in CAM-SIMA will replace both pbuf and state for the purpose of radiatively active aerosol.)rad_aer_*for clarity.This pull request also extends the use of the abstract aerosol interface (
aerosol_propertiesandaerosol_state) into other physics and chemistry code, instead of the previous calls torad_cnst_*.aerosol_propertiesandaerosol_stateare now per-list_idx(climate or diagnostic list) instead of having separate_0listor_rlistfunctions; separate instantiations of the aerosol properties and state objects per(aero_model, list_idx)pair is available. This allows us to keep track of the objects themselves instead oflist_idx. Since most calls are on the climate list (list_idx=0) it will simplify calls and reduce repeating the "0" argument intorad_cnst_*.aerosol_instances_modowns bothaerosol_properties(per model/list) andaerosol_state(per model/list/chunk) as persistent module data. State objects store only pointers to the persistentphys_state(lchnk)andpbufarrays, and is thread-safe. A per-callcreate_states/destroy_statesfactory remains for transient states bound to local copies ofphysics_state(i.e.,microp_aero_run'sstate1).aerosol_instances_modand its use inphyspkg.F90most closely mimics the envisioned implementation under CAM-SIMA whereaerosol_propertiesandaerosol_statebecome inputs to CCPP physics schemes. Under CAM-SIMA the chunk dimension is removed and objects are passed directly — the factory pattern will be retired.This pull request also extracts the creation of optics objects and per-bin sw/lw property calculations from
aerosol_optics_camtoaerosol_optics_core.F90; the latter is fully-portable.After this refactoring, the following modules will be
fully-portable:
aerosol_properties_mod.F90- abstract base class.aerosol_state_mod.F90- abstract base class.radiative_aerosol_definitions.F90- base types for radiatively active aerosol.bulk_aerosol_properties_mod.F90modal_aerosol_properties_mod.F90aerosol_optics_core.F90almost-portable (need to remove
stateandpbufor minor CAM structures):bulk_aerosol_state_mod.F90- remove state and pbuf, use host-model MMR module (see below)radiative_aerosol.F90- use host-model MMR module to resolve indices; diagsaerosol_instances_mod.F90- dechunkize, change init_states to remove pbuf, remove create_states factory once unusedhost-model specific:
aerosol_mmr_cam.F90- pbuf and state%q indexing to replace with host-model specific code (i.e., CAM-SIMA CCPP constituents)needing some future work to separate out dependencies - MAM/CARMA changes were made at a best-effort here and need to be for another day:
modal_aerosol_state_mod.F90: need to CCPPizemodal_aero_wateruptake_drandmodal_aero_calcsize_diag; role ofmodal_aero_datais to be scoped out (out of scope for this work)carma_aerosol_properties_mod.F90: depends oncarma_intr(out of scope for this work)carma_aerosol_state_mod.F90: depends oncarma_intr(out of scope for this work)All changes are bit-for-bit unchanged with CAM (based off
cam6_4_156)List all files eliminated: N/A
List all files added and what they do:
List all existing files that have been modified, and describe the changes: