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

Balance check doesn't check for possible errors in all points #55

Closed
billsacks opened this issue Dec 16, 2017 · 1 comment
Closed

Balance check doesn't check for possible errors in all points #55

billsacks opened this issue Dec 16, 2017 · 1 comment
Assignees
Labels
bfb bit-for-bit enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations

Comments

@billsacks
Copy link
Member

Bill Sacks < [email protected] > - 2017-01-26 09:44:05 -0700
Bugzilla Id: 2415
Bugzilla CC: [email protected], [email protected], [email protected], [email protected],

Leo pointed this out, and I agree with his assessment:

I noticed that the following code from BalanceCheckMod does not check on the maximum error over all columns, merely checks the last column found.
So there is a possibility that the abort is not triggered when it should have been.

   ! Soil energy balance check

   found = .false.
   do c = bounds%begc,bounds%endc
      if (col%active(c)) then
         if (abs(errsoi_col(c)) > 1.0e-6_r8 ) then
            found = .true.
            indexc = c
         end if
      end if
   end do
   if ( found ) then
      write(iulog,*)'WARNING: BalanceCheck: soil balance error (W/m2)'
      write(iulog,*)'nstep         = ',nstep
      write(iulog,*)'errsoi_col    = ',errsoi_col(indexc)
      if (abs(errsoi_col(indexc)) > 1.e-4_r8 .and. (nstep > 2) ) then
         write(iulog,*)'clm model is stopping'
         call endrun(decomp_index=indexc, clmlevel=namec, msg=errmsg(sourcefile, __LINE__))
      end if
   end if

We should change this code (and other code in BalanceCheck mod if there is other code that follows this pattern) to check for the maximum error across all points.

@billsacks billsacks added this to the clm5 milestone Dec 16, 2017
@billsacks billsacks added the enhancement new capability or improved behavior of existing capability label Feb 9, 2018
@billsacks billsacks added the priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations label Nov 7, 2018
@billsacks billsacks removed this from the clm5 milestone Nov 7, 2018
negin513 added a commit to negin513/ctsm that referenced this issue Mar 18, 2019
…d from previous version which was not checking all the columns... Issue ESCOMP#55
negin513 added a commit to negin513/ctsm that referenced this issue Mar 18, 2019
…evious version which was not checking all the columns... Issue ESCOMP#55
negin513 added a commit to negin513/ctsm that referenced this issue Mar 18, 2019
…evious version which was not checking all the columns... Issue ESCOMP#55 (HEAD, balance-check)
negin513 added a commit to negin513/ctsm that referenced this issue Mar 27, 2019
…rrected from previous version which was not checking all the columns... Issue ESCOMP#55 … Tested….Passed…
negin513 added a commit to negin513/ctsm that referenced this issue Mar 27, 2019
… Corrected from previous version which was not checking all the columns... Issue ESCOMP#55 … Tested….Passed…
negin513 added a commit to negin513/ctsm that referenced this issue Mar 27, 2019
…Val and MaxLoc. Corrected from previous version which was not checking all the columns... Issue ESCOMP#55 … Tested….Passed…
negin513 added a commit to negin513/ctsm that referenced this issue Mar 27, 2019
…MaxVal and MaxLoc. Corrected from previous version which was not checking all the columns/patches... Issue ESCOMP#55 … Tested….Passed…
negin513 added a commit to negin513/ctsm that referenced this issue Mar 27, 2019
…MaxLoc. Corrected from previous version which was not checking all the columns/patches... Issue ESCOMP#55 … Tested….Passed…
negin513 added a commit to negin513/ctsm that referenced this issue Mar 27, 2019
…oc. Corrected from previous version which was not checking all the columns and some clean ups.... Issue ESCOMP#55 … Tested….Passed…
billsacks added a commit that referenced this issue Apr 8, 2019
Fix balance check to check for possible errors over all columns/patches

Up until now, the BalanceCheckMod code in biogeophysics did not check
all the possible errors over all columns or patches with the error
threshold for aborting clm and it merely compared the last column/patch
found (where warning threshold is met) with the error threshold.
Therefore, there was always a possibility that the abort is not
triggered when it should have been. This issue was first mentioned in
ctsm issue #55 for soil energy balance check, but a similar issue is
also present for other balance checks in BalanceCheckMod.F90 .

Below is the list of the balance checks with a similar issue: 1. Water
balance check 2. Snow balance check 3. Solar radiation energy balance
check 4. Longwave radiation energy balance check 5. Surface energy
balance check 6. Soil energy balance check (Issue #55)

For remediating this issue, we used MAXVAL and MAXLOC Fortran intrinsic
functions to compare the largest error with the warning and error
thresholds instead of checking the last column or looping over all
columns.

In addition, this PR also makes slight changes to ./run_sys_tests
outputi/case directory structure. Previously for each case in a test
suite, there were two set of directories created by run_sys_tests as
follows: * one directory which included run/ and bld/ and named as
$SCRATCH/[case-name]....  * The case directory which included logs ,
timings, CaseDocs/, and misc. scripts for ./case.build,
./case.submit... under $SCRATCH/tests-[testID]/[case-name]....

This made debugging confusing and populated many folders in user's
scratch.  Now, for every case in a test, all directories related to that
case including bld/ and run/ directories are nested under the case
directory under $SCRATCH/tests-[testID]/[case-name]....  This is now
similar to ./create_test output folder structure.

Resolves ESCOMP/ctsm issue#55
@billsacks
Copy link
Member Author

Fixed by ctsm1.0.dev032

slevis-lmwg added a commit to slevis-lmwg/ctsm that referenced this issue Apr 8, 2019
Fix balance check to check for possible errors over all columns/patches

Up until now, the BalanceCheckMod code in biogeophysics did not check
all the possible errors over all columns or patches with the error
threshold for aborting clm and it merely compared the last column/patch
found (where warning threshold is met) with the error threshold.
Therefore, there was always a possibility that the abort is not
triggered when it should have been. This issue was first mentioned in
ctsm issue ESCOMP#55 for soil energy balance check, but a similar issue is
also present for other balance checks in BalanceCheckMod.F90 .

Below is the list of the balance checks with a similar issue: 1. Water
balance check 2. Snow balance check 3. Solar radiation energy balance
check 4. Longwave radiation energy balance check 5. Surface energy
balance check 6. Soil energy balance check (Issue ESCOMP#55)

For remediating this issue, we used MAXVAL and MAXLOC Fortran intrinsic
functions to compare the largest error with the warning and error
thresholds instead of checking the last column or looping over all
columns.

In addition, this PR also makes slight changes to ./run_sys_tests
outputi/case directory structure. Previously for each case in a test
suite, there were two set of directories created by run_sys_tests as
follows: * one directory which included run/ and bld/ and named as
$SCRATCH/[case-name]....  * The case directory which included logs ,
timings, CaseDocs/, and misc. scripts for ./case.build,
./case.submit... under $SCRATCH/tests-[testID]/[case-name]....

This made debugging confusing and populated many folders in user's
scratch.  Now, for every case in a test, all directories related to that
case including bld/ and run/ directories are nested under the case
directory under $SCRATCH/tests-[testID]/[case-name]....  This is now
similar to ./create_test output folder structure.

Resolves ESCOMP/ctsm issue#55

Conflicts resolved:
	doc/ChangeLog
	doc/ChangeSum
slevis-lmwg pushed a commit to slevis-lmwg/ctsm that referenced this issue Aug 1, 2023
@samsrabin samsrabin added simple bfb bit-for-bit labels Aug 8, 2024
@ekluzek ekluzek moved this to Done (non release/external) in CTSM: Upcoming tags Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations
Projects
Status: Done (non release/external)
Development

No branches or pull requests

3 participants