Implements cam_thermo_water_update and CCPPized check_energy #316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…m stash Needs work to split into cam_thermo_water_update. This is WIP code
…or divergence from CAM and/or -SIMA
total_hours_in_debugging_one_line = 6
Squashed commit of the following: commit 8b8a8e0 Fix merge conflicts commit bef25fb Update atmos_phys external commit 240ef54 Set wv_idx in air_composition total_hours_in_debugging_one_line = 6 commit 38fdbb2 Read cp_or_cv_dycore and identify dycore information from CAM snapshot commit 8fca3a4 Update vcoord to energy_formula commit 0d16be9 New const_get_index logic without cam_ccpp_cap dependency commit e13e7ea Fix build (part 1); update standard names and atmos_phys external commit c015e28 Update ncar-physics external commit 2372f7f Fix for b4b to CAM-SIMA (w/ History); add notes on modifications and/or divergence from CAM and/or -SIMA commit 4ce8427 Add gmean_mod to src/utils including support infra in physics_grid.F90 commit 7493a1d Fix build issues; update factor in get_cp call to 1/dp commit 041cdfb Add is_first_timestep registry commit 333ad4e Initial port of updates to energy budget (port of CAM #761) into CAM-SIMA commit 8648970 Initial implementation of cp_to_cv_dycore/cam_thermo_water_update from stash Needs work to split into cam_thermo_water_update. This is WIP code
This was referenced Oct 25, 2024
…ycore initialization
nusbaume
requested changes
Dec 11, 2024
There was a problem hiding this comment.
Looks good @jimmielin! I did have some change requests, but hopefully most of them are minor (and if any do become problematic please let me know). Thanks!
nusbaume
reviewed
Dec 12, 2024
src/data/air_composition.F90
Outdated
| ! for compatibility with CAM-SIMA that allocates thermodynamic_active_species_idx(0:num_advected) | ||
| ! (whereas CAM only allocates 1-indexed) I subset it here. This should be verified during code | ||
| ! review. | ||
| ! Note: species index subset to 1: because SIMA currently uses index 0. See #334. |
There was a problem hiding this comment.
Might be good to make it clear that #334 is a Github issue:
Suggested change
| ! Note: species index subset to 1: because SIMA currently uses index 0. See #334. | |
| ! Note: species index subset to 1: because SIMA currently uses index 0. See Github issue #334 in ESCOMP/CAM-SIMA. |
PeterHjortLauritzen
approved these changes
Dec 12, 2024
src/data/air_composition.F90
Outdated
| ! inv_cp: output inverse cp instead of cp | ||
| logical, intent(in) :: inv_cp | ||
| real(kind_phys), intent(out) :: cp(:,:) | ||
| ! dp: if provided then tracer is mass not mixing ratio |
There was a problem hiding this comment.
Factor is used to convert tracer to dry mixing ratio. So if tracer is mass then one can divide by dpdry (factor=dpdry) to get dry mixing ratio. If tracer is moist mixing ratio then factor is used to convert wet to dry mixing ratio (dpmoist/dpdry).
I guess a more accurate comment would be:
factor to convert tracer to dry mixing ratio.
jimmielin
commented
Dec 12, 2024
src/dynamics/se/dp_coupling.F90
Outdated
Comment on lines
787
to
801
| ! **TEMP** TODO CHECK hplin: CAM has this if-clause for dry_air_species_num > 0 | ||
| ! or otherwise uses zvirv = zvir. CAM-SIMA previously did not have this, and | ||
| ! instead has a switch for update_thermodynamic_variables. Check if we still want | ||
| ! this if-clause or change it to something else. | ||
| if (dry_air_species_num > 0) then | ||
| call cam_thermo_dry_air_update( & | ||
| mmr = const_data_ptr, & ! dry MMR | ||
| T = phys_state%t, & | ||
| ncol = pcols, & | ||
| pver = pver, & | ||
| update_thermo_variables = cam_runtime_opts%update_thermodynamic_variables() & | ||
| ) | ||
| else | ||
| zvirv(:,:) = zvir | ||
| end if |
There was a problem hiding this comment.
Hi @PeterHjortLauritzen and @nusbaume, could you confirm this change (to bring it consistent with CAM behavior) is correct? The original CAM-SIMA call was simply
call cam_thermo_update(const_data_ptr, phys_state%t, pcols, &
cam_runtime_opts%update_thermodynamic_variables())Thanks!
nusbaume
approved these changes
Dec 12, 2024
There was a problem hiding this comment.
Everything looks good to me now! I did have a few final change requests, but none that require an extra review from me. I also left some of my comments un-resolved if they eventually might need additional input from Peter. Thanks again for getting this into CAM-SIMA!
src/data/air_composition.F90
Outdated
| ! | ||
| real(kind_phys), intent(in) :: tracer(:,:,:) | ||
| ! inv_cp: output inverse cp instead of cp | ||
| logical, intent(in) :: inv_cp | ||
| real(kind_phys), intent(out) :: cp(:,:) | ||
| ! dp: if provided then tracer is mass not mixing ratio | ||
| ! if provided then tracer is not a mass mixing ratio |
There was a problem hiding this comment.
Might want to specify that is a dry mass mixing ratio (?):
Suggested change
| ! if provided then tracer is not a mass mixing ratio | |
| ! if provided then tracer is not a dry mass mixing ratio |
src/data/cam_thermo.F90
Outdated
| @@ -240,7 +241,10 @@ subroutine cam_thermo_dry_air_update(mmr, T, ncol, update_thermo_variables, to_d | |||
|
|
|||
| if (present(to_dry_factor)) then | |||
| if (SIZE(to_dry_factor, 1) /= ncol) then | |||
| call endrun(subname//'DIM 1 of to_dry_factor is'//to_str(SIZE(to_dry_factor,1))//'but should be'//to_str(ncol)) | |||
| call endrun(subname//'DIM 1 of to_dry_factor is'//stringify((/SIZE(to_dry_factor,1)/))//'but should be'//stringify((/ncol/))) | |||
There was a problem hiding this comment.
I think you might want to add spaces here:
Suggested change
| call endrun(subname//'DIM 1 of to_dry_factor is'//stringify((/SIZE(to_dry_factor,1)/))//'but should be'//stringify((/ncol/))) | |
| call endrun(subname//'DIM 1 of to_dry_factor is '//stringify((/SIZE(to_dry_factor,1)/))//' but should be '//stringify((/ncol/))) |
I have this same request for the other related endrun calls below as well.
kuanchihwang
approved these changes
Dec 13, 2024
Co-authored-by: Kuan-Chih Wang <[email protected]>
Co-authored-by: Kuan-Chih Wang <[email protected]>
Co-authored-by: Kuan-Chih Wang <[email protected]>
Co-authored-by: Kuan-Chih Wang <[email protected]>
|
Regression tests completed Test failures for this tag: |
kuanchihwang
added a commit
to kuanchihwang/CAM-SIMA
that referenced
this pull request
Dec 16, 2024
* Adjust wording and keep code comments up-to-date. * Fix up code style inconsistencies.
kuanchihwang
added a commit
to kuanchihwang/CAM-SIMA
that referenced
this pull request
Dec 16, 2024
* Adjust wording and keep code comments up-to-date * Concentrate all calls to `mark_as_initialized` in one place * Fix up code style inconsistencies
kuanchihwang
added a commit
to kuanchihwang/CAM-SIMA
that referenced
this pull request
Dec 16, 2024
* Adjust wording and keep code comments up-to-date * Concentrate all calls to `mark_as_initialized` in one place * Fix up code style inconsistencies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Originator(s): jimmielin
Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):
All changes are bit-for-bit, except those noted:
Implements
cam_thermo_water_update:cp_or_cv_dycore(specific_heat_for_air_used_in_dycore) inair_composition.F90for SE and MPAS dynamical corescp_or_cv_dycorefrom CAM snapshot (refer to companion CAM PR)dyn_grid.F90by looking at global attributes of the initial file; seefind_energy_formulais_first_timesteplogical state flagPorts
cam_thermoand related updates inair_compositionanddp_couplingfrom CAM 6.3.109 (https://github.com/ESCOMP/CAM/pull/761/files)get_cp,get_Rinair_composition.F90to use moist mixing ratiosdp_coupling::derived_phys_dryto account for all water tracers instead of just Qcappavinstead ofcappaChanges
vcoordindyn_tests_utils(old CAM) toenergy_formulanow incam_thermo_formula(separated out into a different file to avoid dependency issues)vc_moist_pressureis nowENERGY_FORMULA_DYCORE_FV;vc_dry_pressureis_SE;vc_heightis_MPASPorts global mean utility module (
gmean_mod.F90), de-chunkized from CAM:get_wghtinphysics_gridfor weighted sum calculationImports
check_energy_chngandcheck_energy_fixfromatmospheric_physicsDescribe any changes made to build system: N/A
Describe any changes made to the namelist: contained within ncar-physics
List any changes to the defaults for the input datasets (e.g. boundary datasets):
cp_or_cv_dycorein CAM snapshotsList all files eliminated and why: N/A
List all files added and what they do:
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>)Note: bit-for-bit in check_energy with CAM is tricky to validate without dycore updates to SE; may need to merge #301 first