Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meier2022 option fails in BiogeophysPreFluxCalcsMod.F90 line 208 in IHist case #2264

Closed
slevis-lmwg opened this issue Nov 26, 2023 · 3 comments · Fixed by #2271
Closed

Meier2022 option fails in BiogeophysPreFluxCalcsMod.F90 line 208 in IHist case #2264

slevis-lmwg opened this issue Nov 26, 2023 · 3 comments · Fixed by #2271
Assignees
Labels
priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations

Comments

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Nov 26, 2023

Brief summary of bug

Meier2022 option fails on Dec 31, 2001, of this IHist case:
NCAR/LMWG_dev#38
Simulation started in 1989.

General bug information

CTSM version you are using:
ctsm5.1.dev154

Does this bug cause significantly incorrect results in the model's science?
Run fails.

Configurations affected:
./create_newcase --case ctsm51d154_2deg_GSWP3V1_hist --compset HIST_DATM%GSWP3v1_CLM51%BGC-CROP_SICE_SOCN_MOSART_SGLC_SWAV --res f19_g17 --run-unsupported

Details of bug

During pre-tag testing for dev154, I encountered the same failure in the aux_clm test-suite. I resolved it by adding the if istcrop statement:

               if ( htop(p) <= 1.e-10_r8 )then
                  if (lun%itype(l) == istcrop) then
                     z0m(p) = 0._r8
                     displa(p) = 0._r8
                  else
                     write(iulog,*) ' nstep = ', get_nstep(), ' htop = ', htop(p)
                     call endrun(subgrid_index=p, subgrid_level=subgrid_level_patch, msg=errMsg(sourcefile, __LINE__))
                  end if
               else
                   z0m(p) = htop(p) * (1._r8 - displa(p) / htop(p)) * exp(-0.4_r8 * U_ustar + &
                               log(pftcon%z0v_cw(patch%itype(p))) - 1._r8 + pftcon%z0v_cw(patch%itype(p))**(-1._r8))
               end if

That this now fails in an IHist simulation tells me that the model likely encounters tiny htop for a newly introduced non-crop patch. I propose that we remove the new if istcrop, as well as the endrun and just set z0m and displa to 0 for tiny htop.

Important details of your setup / configuration so we can reproduce the bug

Case was submitted here:
/glade/work/slevis/git_ctsm_tags/ctsm5.1.dev154/cime/scripts/ctsm51d154_2deg_GSWP3V1_hist

Important output or errors that show the problem

Case ran here:
/glade/scratch/slevis/ctsm51d154_2deg_GSWP3V1_hist/run

@slevis-lmwg slevis-lmwg self-assigned this Nov 26, 2023
@slevis-lmwg slevis-lmwg added tag: bug - critical priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Nov 26, 2023
@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Nov 27, 2023

My test (i.e. removing the if istcrop and the endrun, as described above) and restarting on Jan 1, 2001, worked. The simulation finished in 2014 as intended.

At stand-up with @samsrabin @ekluzek
we agreed that I test reverting the code back to original in BiogeophysPreFluxCalcsMod.F90
and change subr. InitCold in CanopyStateType.F90 like this:

-       this%htop_patch(p)        = 0._r8
+       this%htop_patch(p)        = 0.01_r8

A Clm50 test in the cheyenne test-suite already shows diffs greater than roundoff in 357 fields.

To maintain bfb same answers in clm50 and clm45, I am now trying this:

-       this%htop_patch(p)        = 0._r8
+       select case (z0param_method)
+       case ('ZengWang2007')
+          this%htop_patch(p)     = 0._r8  ! to preserve bfb in clm50 and clm45
+       case ('Meier2022')
+          this%htop_patch(p)     = 0.01_r8
+       case default
+          call endrun(msg="ERROR canopystate InitCold: unknown z0param_method "//errmsg(sourcefile, __LINE__))
+       end select

Only Clm51 tests show diffs, as expected. Diffs are small.

I also restarted (at 2001 again) the longer IHist simulation that originally revealed the problem (linked at the top of the page). Expecting changed answers, but then try the beginning of the run, too (expecting bfb).

@ekluzek suggested adding (or changing) an IHist test that runs across the 2001-2002 transition, so as to catch this in the future. I don't think I agree because we are soon changing to ctsm5.2 datasets, and we do not know whether the same problem will appear in the same year.

@samsrabin suggested functionalizing the return of htop_min that currently occurs in CNVegStructUpdate. Open issue?

@ekluzek
Copy link
Collaborator

ekluzek commented Nov 28, 2023

@slevis-lmwg this is really close to #2234 if they are duplicates we should close one or the other. Since, you are actively working on this one, I think that might mean closing #2234 as a duplicate of this. Can you look at them and figure that out? Thanks

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Nov 29, 2023

Our second attempt explained here does not fix the problem, presumably because initializing htop in InitCold does not initialize it at year transitions. Also, from the conversation with @samsrabin on Monday, crop htop may equal zero before emergence and after harvest. So I will return to my original fix, open a PR, and see what people think.

@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants