Skip to content

Clubb coriolis#19

Open
HingOng wants to merge 38 commits intolarson-group:masterfrom
HingOng:clubb_Coriolis
Open

Clubb coriolis#19
HingOng wants to merge 38 commits intolarson-group:masterfrom
HingOng:clubb_Coriolis

Conversation

@HingOng
Copy link
Contributor

@HingOng HingOng commented Aug 6, 2025

These changes implement the nontraditional and traditional Coriolis terms, each controlled by a flag. The nontraditional ones are explicitly integrated in the up2, wp2, and upwp equations, and so are the traditional ones in the upwp and vpwp equations. The Coriolis parameters are either passed through the host model interface or calculated from the latitude in the standalone namelist. The Coriolis terms are ignored by the default flag setting.

With the default setting, the only change is the setting of the traditional Coriolis parameter from the standalone namelist. Previous versions allow user to set a Coriolis parameter inconsistent with the latitude. The present version ignores the "fcor_nl" and calculates "fcor" from "lat_vals".

The implementation of the Coriolis terms has been validated with an analytical benchmark.

LarsonGroupSysAdmin and others added 16 commits June 5, 2024 01:58
…g it with an unstructured data statement. I added this due as a fix before thinking qclvar was a local variable, which happened because I mixed up the CLUBB_CAM and CLUBBND_CAM flags. larson-group/cam#175
…contains pointers, and these pointers will be different on the host and device, so it is bad in theory to copy the structure back to the CPU as it might overwrite cpu memory pointers with gpu memory pointers. In practice though I've seen no problems caused by this, I'm just making this commit preemptively, and it has already been tested with the ECT test.
Copy link
Contributor

@vlarson vlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this pull request a superset of #20 ?

Do these code changes leave the results BFB identical when the flags are set to zero?

I can make these code changes if you don't have time or interest.

'vp2_bt', 'vp2_ma', 'vp2_ta', 'vp2_tp', 'vp2_dp1', 'vp2_dp2',
'vp2_pr1', 'vp2_pr2', 'vp2_sdmp', 'vp2_cl', 'vp2_pd', 'vp2_sf', 'vp2_splat',
'up2_bt', 'up2_ma', 'up2_ta', 'up2_tp', 'up2_dp1', 'up2_dp2',
'up2_bt', 'up2_ma', 'up2_ta', 'up2_tp', 'up2_dp1', 'up2_dp2', 'up2_nct',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,312 @@
commit 51107487c8ae0f96461e5a882a203d3019aa6aa5 (HEAD -> clubb_Coriolis_cam, newremote.00/clubb_Coriolis_cam)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't need to commit this file because it's automatically generated.

!$acc end data
!$acc end data

#ifdef CLUBB_CAM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hing thinks that this was included by accident, but he hasn't tested the code on a GPU, so he's not 100% sure.

@@ -0,0 +1,110 @@
! $Id$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coriolis_test_parameters.in shuts off all the pressure terms so that Hing can compare with his analytic solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider creating a Jenkins test that runs the Coriolis test and compares to the analytic answer.

@@ -0,0 +1,288 @@
commit 46cac2787c3802be0418762948ad6d1231dde182
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, we won't need to commit this file because it's automatically generated.

fcor_nl
fcor_nl ! obsolete
! fcor_nl can be read from a namelist but is not used for any calculation.
! fcor and fcory are calculated from lat_vals from a namelist.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is fcor always generated from lat_vals? I think we want to retain the capability to directly specify fcor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of this change, we don't expect the results to be BFB. We'll have to see how much the answers change. If not much, then we could delete fcor_nl. But if the answers do change, then maybe we do want to keep the option of specifying fcor, just so that we match our benchmark LES better.

sens_ht = 0._core_rknd
latent_ht = 0._core_rknd
fcor_nl = 1.e-4_core_rknd
fcor_nl = 1.e-4_core_rknd ! obsolete
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you make this obsolete?

p_sfc = p_sfc_nl
T_sfc = T_sfc_nl
fcor = fcor_nl
fcor = two * omega_planet * sin ( lat_vals*radians_per_deg ) ! Retire fcor_nl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we want this to be optional, if fcor_nl is not specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have run a cgils_s6 test with this line restored as "fcor = fcor_nl" and compared the results against those from the "larson-group:master" branch. They match BFB.

A problem is that cgils_s6 test is extremely sensitive to fcor.
If prescribed from fcor_nl, fcor = 4.264000000000000E-005, the cgils_s6 test completes normally.
If calculated from lat_vals, fcor = 4.263948942216714E-005, the cgils_s6 test aborts with the following error:
iteration = 118; time = 35400.0
Fatal error in CLUBB - MPI Process 1 / Chunk index 1
Column index not av
ailable. Latitude/Longitude for column 1: 17.00/******
Latitude range from -90
to 90 degrees, Longitude range from -180 to 180 degrees
mixt_frac * precip_frac_1 + ( 1 - mixt_frac ) * precip_frac_2 /= precip_frac wi
thin numerical roundoff
level = 15
mixt_frac * precip_frac_1 + ( 1 - mixt_frac ) * precip_frac_2 =
8.254983776519802E-002
precip_frac = 8.254983776519802E-002
Fatal error in CLUBB in - MPI Process 1 / Chunk index 1
Grid column index i
= 1, Latitude 17.00 / Longitude 17.00
Latitude range from -90 to 90 degrees
, Longitude range from -180 to 180 degrees
precip_frac_assert_check failed
Fatal error in CLUBB - MPI Process 1 / Chunk index 1
Column index not av
ailable. Latitude/Longitude for column 1: 17.00/******
Latitude range from -90
to 90 degrees, Longitude range from -180 to 180 degrees
in setup_pdf_parameters after calling precip_fraction
Fatal error in CLUBB - MPI Process 1 / Chunk index 1
Column index not av
ailable. Latitude/Longitude for column 1: 17.00/******
Latitude range from -90
to 90 degrees, Longitude range from -180 to 180 degrees
Fatal error after setup_pdf_parameters_api
Fatal error in CLUBB - MPI Process 1 / Chunk index 1
Column index not av
ailable. Latitude/Longitude for column 1: 17.00/******
Latitude range from -90
to 90 degrees, Longitude range from -180 to 180 degrees
Fatal error after pdf_hydromet_microphys_prep
CLUBB-TIMER time_loop_init = 0.0022
CLUBB-TIMER time_clubb_advance = 0.0456
CLUBB-TIMER time_clubb_pdf = 0.0283
CLUBB-TIMER time_SILHS = 0.0001
CLUBB-TIMER time_microphys_scheme = 0.0103
CLUBB-TIMER time_microphys_advance = 0.0171
CLUBB-TIMER time_loop_end = 0.1028
CLUBB-TIMER time_output_multi_col = 0.0001
CLUBB-TIMER time_total = 0.2073
Fatal error in clubb, check your parameter values and timestep

I had to reduce the timestep to normally complete the run with calculated fcor.

do k = 1, gr%nzm
do i = 1, ngrdcol
upwp(i,k) = zero ! vertical u momentum flux
upwp(i,k) = upwp_init(k) ! vertical u momentum flux
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is upwp_init initialized for all cases? Usually, it should be zero, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upwp_init is set automatically from upwp in clubb_driver.F90.

@vlarson
Copy link
Contributor

vlarson commented Aug 7, 2025

Is this pull request a superset of #20 ?

No, #20 is a pull request for the CAM tag. #19 is a pull request for CLUBB master.

Do these code changes leave the results BFB identical when the flags are set to zero?

Hing hasn't done this test.

em = em + 1.5_core_rknd * w_tol_sqd
do i = 1, ngrdcol
do k=1,gr%nzm
upwp(i,k) = 0.5_core_rknd * dt_main * fcory(i) * ( up2(i,k) - wp2(i,k) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where upwp_init is set.

HingOng and others added 10 commits August 8, 2025 14:53
Rename fcory as fcor_y and add description
Rename fcory as fcor_y and add description
Rename fcory as fcor_y and add description
Rename fcory as fcor_y and add description
Rename fcory as fcor_y and add description
Rename fcory as fcor_y and add description
Rename fcor as fcor_y and add description.
Also, add description for the coriolis_test
Rename fcory as fcor_y and add description.
Add description for the Coriolis test case
@HingOng HingOng requested a review from vlarson November 26, 2025 08:29
@HingOng
Copy link
Contributor Author

HingOng commented Nov 26, 2025

This branch is merged with up-to-date larson-group/clubb_release/master
When l_nontraditional_Coriolis and l_traditional_Coriolis are .false. (default), this merged version outputs additional variables up2_nct, wp2_nct, upwp_nct, upwp_tct, and vpwp_tct to *_zm.nc and wp3_nct and wp2up to *_zt.nc. However, the other output variables remain identical with those generated by larson-group/clubb_release/master for the bomex case.
When l_nontraditional_Coriolis and l_traditional_Coriolis are .true., this merged version outputs an additional flag l_hole_fill = "T". However, the other output variables remain identical with the unmerged version of this branch for the atex_long case (known to be sensitive to the nontraditional Coriolis terms).

@vlarson vlarson requested a review from huebleruwm November 26, 2025 12:53
@vlarson
Copy link
Contributor

vlarson commented Dec 2, 2025

@HingOng, do you happen to have code changes for clubb_intr.F90 in CAM that correspond to these code changes to CLUBB?

@vlarson
Copy link
Contributor

vlarson commented Dec 2, 2025

@HingOng, with the newest version of your branch, do the budgets balance, according to this script?:

https://github.com/larson-group/clubb/blob/master/postprocessing/check_budgets_balance/checkBudget.py

@HingOng
Copy link
Contributor Author

HingOng commented Dec 2, 2025

@HingOng, do you happen to have code changes for clubb_intr.F90 in CAM that correspond to these code changes to CLUBB?

Yes, I have them on a local branch.

@HingOng
Copy link
Contributor Author

HingOng commented Dec 2, 2025

@HingOng, with the newest version of your branch, do the budgets balance, according to this script?:

https://github.com/larson-group/clubb/blob/master/postprocessing/check_budgets_balance/checkBudget.py

Yes, checkBudget.py returns "Budgets successfully balance!"

@vlarson
Copy link
Contributor

vlarson commented Dec 2, 2025

@HingOng, do you happen to have code changes for clubb_intr.F90 in CAM that correspond to these code changes to CLUBB?

Yes, I have them on a local branch.

Is the branch on github? Or is there another way to share it?

@HingOng
Copy link
Contributor Author

HingOng commented Dec 2, 2025

@HingOng, do you happen to have code changes for clubb_intr.F90 in CAM that correspond to these code changes to CLUBB?

Yes, I have them on a local branch.

Is the branch on github? Or is there another way to share it?

I just pushed the branch to my fork of CAM:
https://github.com/HingOng/CAM/tree/clubb_Coriolis

Note that this branch is based on an outdated version of CAM and CLUBB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants